From ef61ab684571a0ae9eeaf80290520331cc21f129 Mon Sep 17 00:00:00 2001
From: Tin Tvrtkovic <tin@high.rs>
Date: Thu, 28 Jan 2021 02:35:25 +0100
Subject: [PATCH] Fix #118

---
 HISTORY.rst                                   |  2 +
 src/cattr/converters.py                       | 39 ++++++++++---------
 tests/metadata/test_genconverter.py           | 27 +++++++++++++
 ...tance.py => test_converter_inheritance.py} |  9 +++--
 4 files changed, 56 insertions(+), 21 deletions(-)
 rename tests/{test_genconverter_inheritance.py => test_converter_inheritance.py} (57%)

diff --git a/HISTORY.rst b/HISTORY.rst
index 98835255..7f007914 100644
--- a/HISTORY.rst
+++ b/HISTORY.rst
@@ -9,6 +9,8 @@ History
   (`#115 <https://github.com/Tinche/cattrs/pull/115>`_)
 * Fix `GenConverter` behavior with inheritance hierarchies of `attrs` classes.
   (`#117 <https://github.com/Tinche/cattrs/pull/117>`_) (`#116 <https://github.com/Tinche/cattrs/issues/116>`_)
+* Refactor `GenConverter.un/structure_attrs_fromdict` into `GenConverter.gen_un/structure_attrs_fromdict` to allow calling back to `Converter.un/structure_attrs_fromdict` without sideeffects.
+  (`#118 https://github.com/Tinche/cattrs/issues/118`_)
 
 1.1.2 (2020-11-29)
 ------------------
diff --git a/src/cattr/converters.py b/src/cattr/converters.py
index d393639d..e8505b72 100644
--- a/src/cattr/converters.py
+++ b/src/cattr/converters.py
@@ -1,18 +1,6 @@
 from enum import Enum
 from functools import lru_cache
-from typing import (  # noqa: F401, imported for Mypy.
-    Any,
-    Callable,
-    Dict,
-    FrozenSet,
-    Mapping,
-    Optional,
-    Sequence,
-    Set,
-    Tuple,
-    Type,
-    TypeVar,
-)
+from typing import Any, Callable, Dict, Mapping, Optional, Tuple, Type, TypeVar
 
 from attr import fields, resolve_types
 
@@ -291,7 +279,8 @@ def _structure_default(self, obj, cl):
         )
         raise ValueError(msg)
 
-    def _structure_call(self, obj, cl):
+    @staticmethod
+    def _structure_call(obj, cl):
         """Just call ``cl`` with the given ``obj``.
 
         This is just an optimization on the ``_structure_default`` case, when
@@ -314,7 +303,7 @@ def structure_attrs_fromtuple(
 
         return cl(*conv_obj)  # type: ignore
 
-    def _structure_attr_from_tuple(self, a, name, value):
+    def _structure_attr_from_tuple(self, a, _, value):
         """Handle an individual attrs attribute."""
         type_ = a.type
         if type_ is None:
@@ -452,7 +441,8 @@ def _structure_tuple(self, obj, tup: Type[T]):
                 for t, e in zip(tup_params, obj)
             )
 
-    def _get_dis_func(self, union):
+    @staticmethod
+    def _get_dis_func(union):
         # type: (Type) -> Callable[..., Type]
         """Fetch or try creating a disambiguation function for a union."""
         union_types = union.__args__
@@ -491,7 +481,20 @@ def __init__(
         self.omit_if_default = omit_if_default
         self.type_overrides = type_overrides
 
-    def unstructure_attrs_asdict(self, obj: Any) -> Dict[str, Any]:
+        if unstruct_strat is UnstructureStrategy.AS_DICT:
+            # Override the attrs handler.
+            self._unstructure_func.register_func_list(
+                [
+                    (_is_attrs_class, self.gen_unstructure_attrs_fromdict),
+                ]
+            )
+            self._structure_func.register_func_list(
+                [
+                    (_is_attrs_class, self.gen_structure_attrs_fromdict),
+                ]
+            )
+
+    def gen_unstructure_attrs_fromdict(self, obj: Any) -> Dict[str, Any]:
         attribs = fields(obj.__class__)
         if any(isinstance(a.type, str) for a in attribs):
             # PEP 563 annotations - need to be resolved.
@@ -513,7 +516,7 @@ def unstructure_attrs_asdict(self, obj: Any) -> Dict[str, Any]:
         )
         return h(obj)
 
-    def structure_attrs_fromdict(
+    def gen_structure_attrs_fromdict(
         self, obj: Mapping[str, Any], cl: Type[T]
     ) -> T:
         attribs = fields(cl)
diff --git a/tests/metadata/test_genconverter.py b/tests/metadata/test_genconverter.py
index 83fc4dcd..82284c6e 100644
--- a/tests/metadata/test_genconverter.py
+++ b/tests/metadata/test_genconverter.py
@@ -172,3 +172,30 @@ def test_type_overrides(cl_and_vals):
                         assert field.name not in unstructured
                 elif field.default == val:
                     assert field.name not in unstructured
+
+
+def test_calling_back():
+    """Calling unstructure_attrs_asdict from a hook should not override a manual hook."""
+    converter = Converter()
+
+    @attr.define
+    class C:
+        a: int = attr.ib(default=1)
+
+    def handler(obj):
+        return {
+            "type_tag": obj.__class__.__name__,
+            **converter.unstructure_attrs_asdict(obj),
+        }
+
+    converter.register_unstructure_hook(C, handler)
+
+    inst = C()
+
+    expected = {"type_tag": "C", "a": 1}
+
+    unstructured1 = converter.unstructure(inst)
+    unstructured2 = converter.unstructure(inst)
+
+    assert unstructured1 == expected, repr(unstructured1)
+    assert unstructured2 == unstructured1, repr(unstructured2)
diff --git a/tests/test_genconverter_inheritance.py b/tests/test_converter_inheritance.py
similarity index 57%
rename from tests/test_genconverter_inheritance.py
rename to tests/test_converter_inheritance.py
index b38d3694..17c32324 100644
--- a/tests/test_genconverter_inheritance.py
+++ b/tests/test_converter_inheritance.py
@@ -1,8 +1,11 @@
-from cattr.converters import GenConverter
 import attr
+import pytest
 
+from cattr.converters import Converter, GenConverter
 
-def test_inheritance():
+
+@pytest.mark.parametrize("converter_cls", [GenConverter, Converter])
+def test_inheritance(converter_cls):
     @attr.s(auto_attribs=True)
     class A:
         i: int
@@ -11,7 +14,7 @@ class A:
     class B(A):
         j: int
 
-    converter = GenConverter()
+    converter = converter_cls()
 
     # succeeds
     assert A(1) == converter.structure(dict(i=1), A)