diff --git a/HISTORY.md b/HISTORY.md index a4464ea6..1824aba0 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -13,6 +13,8 @@ Our backwards-compatibility policy can be found [here](https://github.com/python ## NEXT +- Fix recursive attrs unstructuring in the msgspec converter so nested instances honor custom hooks and retain private attributes. + ([#795](https://github.com/python-attrs/cattrs/issues/795)) - Dict unstructuring hook factories now inherit the converter's `omit_if_default` setting when `_cattrs_omit_if_default` is omitted. Explicit `True` or `False` values and per-attribute overrides retain their precedence. ([#238](https://github.com/python-attrs/cattrs/issues/238)) - Support the [`frozendict`](https://docs.python.org/3.15/builtins/stdtypes.html#frozendict) built-in on Python 3.15+. diff --git a/src/cattrs/preconf/msgspec.py b/src/cattrs/preconf/msgspec.py index 1c73d28f..b8193fd4 100644 --- a/src/cattrs/preconf/msgspec.py +++ b/src/cattrs/preconf/msgspec.py @@ -200,18 +200,27 @@ def msgspec_attrs_unstructure_factory( resolve_types(type) attribs = fields(base) - if msgspec_skips_private and any(attr.name.startswith("_") for attr in attribs): - # Private attributes we have to do ourselves. - return converter.gen_unstructure_attrs_fromdict(type) - try: working_set = _already_probing.working_set - if base in working_set: - return to_builtins except AttributeError: + working_set = None + + if working_set is not None and base in working_set: + return to_builtins + + if msgspec_skips_private and any(attr.name.startswith("_") for attr in attribs): + # Private attributes we have to do ourselves. Defer hook selection while + # another attrs type is being probed, since generating here could capture + # a temporary recursive fallback for one of its fields. + if working_set: + return converter.unstructure + return converter.gen_unstructure_attrs_fromdict(type) + + if working_set is None: working_set = set() _already_probing.working_set = working_set + needs_cattrs_hook = False try: working_set.add(base) if any( @@ -219,12 +228,19 @@ def msgspec_attrs_unstructure_factory( not in (identity, to_builtins) for attr in attribs ): - return converter.gen_unstructure_attrs_fromdict(type) + needs_cattrs_hook = True finally: working_set.remove(base) if not working_set: del _already_probing.working_set + if needs_cattrs_hook: + # Generate only after the outermost probe finishes so field handlers + # cannot capture the temporary to_builtins recursion fallback. + if working_set: + return converter.unstructure + return converter.gen_unstructure_attrs_fromdict(type) + return to_builtins diff --git a/tests/preconf/test_msgspec_cpython.py b/tests/preconf/test_msgspec_cpython.py index 1576c039..11365699 100644 --- a/tests/preconf/test_msgspec_cpython.py +++ b/tests/preconf/test_msgspec_cpython.py @@ -3,6 +3,7 @@ from __future__ import annotations from dataclasses import dataclass +from datetime import datetime, timezone from enum import Enum from typing import ( Any, @@ -20,7 +21,7 @@ from attrs import define from hypothesis import given from msgspec import Struct, to_builtins -from pytest import fixture +from pytest import fixture, raises from cattrs.fns import identity from cattrs.preconf.json import make_converter as make_json_converter @@ -49,6 +50,33 @@ class C: _a: int +@define +class RecursiveEvent: + at: datetime + children: list[RecursiveEvent] + + +@define +class RecursiveA: + bs: list[RecursiveB] + + +@define +class RecursiveB: + _secret: int + as_: list[RecursiveA] + + +@define +class RecursivePlain: + children: list[RecursivePlain] + + +@define +class ProbeFailureAttrs: + value: datetime + + @dataclass class DataclassA: a: int @@ -172,6 +200,72 @@ class B(Struct): assert converter.structure({"b": 1}, B) == B(1) +def test_unstructure_recursive_attrs_uses_custom_hooks(converter: Conv): + """Custom hooks are used at every depth of recursive attrs classes.""" + converter.register_unstructure_hook(datetime, lambda value: int(value.timestamp())) + timestamp = datetime(2026, 1, 1, tzinfo=timezone.utc) + + assert converter.unstructure( + RecursiveEvent( + timestamp, [RecursiveEvent(timestamp, [RecursiveEvent(timestamp, [])])] + ) + ) == { + "at": 1767225600, + "children": [ + {"at": 1767225600, "children": [{"at": 1767225600, "children": []}]} + ], + } + + +def test_unstructure_mutually_recursive_attrs_preserves_private_fields(converter: Conv): + """Private fields are retained through mutually recursive attrs classes.""" + + value = RecursiveA([RecursiveB(1, [RecursiveA([RecursiveB(2, [])])])]) + + assert converter.unstructure(value) == { + "bs": [{"_secret": 1, "as_": [{"bs": [{"_secret": 2, "as_": []}]}]}] + } + + reverse_root = RecursiveB(3, [RecursiveA([RecursiveB(4, [])])]) + assert make_converter().unstructure(reverse_root) == { + "_secret": 3, + "as_": [{"bs": [{"_secret": 4, "as_": []}]}], + } + + +def test_unstructure_plain_recursive_attrs_still_uses_msgspec(converter: Conv): + """Recursive attrs types without custom fields retain the passthrough path.""" + + value = RecursivePlain([RecursivePlain([])]) + + assert converter.get_unstructure_hook(RecursivePlain) is to_builtins + assert converter.unstructure(value) == {"children": [{"children": []}]} + + +def test_unstructure_probe_state_is_cleared_after_hook_error( + converter: Conv, monkeypatch +): + """A failed hook lookup doesn't poison later converter use.""" + + converter.register_unstructure_hook(datetime, lambda value: int(value.timestamp())) + get_hook = converter.get_unstructure_hook + + def fail_for_datetime(type, *args, **kwargs): + if type is datetime: + raise RuntimeError("probe failed") + return get_hook(type, *args, **kwargs) + + monkeypatch.setattr(converter, "get_unstructure_hook", fail_for_datetime) + with raises(RuntimeError, match="probe failed"): + converter.get_unstructure_hook(ProbeFailureAttrs) + + monkeypatch.setattr(converter, "get_unstructure_hook", get_hook) + assert converter.get_unstructure_hook(ProbeFailureAttrs) is not to_builtins + assert converter.unstructure( + ProbeFailureAttrs(datetime(2026, 1, 1, tzinfo=timezone.utc)) + ) == {"value": 1767225600} + + @given(simple_typed_classes(text_codec="ascii", allow_infinity=False, allow_nan=False)) def test_simple_classes(cls_and_vals): cl, posargs, kwargs = cls_and_vals