diff --git a/HISTORY.rst b/HISTORY.rst index 78893c50..8dde8be5 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -2,6 +2,11 @@ History ======= +1.1.2 (unrelased) +----------------- +* Disambigualation do not pick non required field. + (`#108 `_) + 1.1.1 (2020-10-30) ------------------ * Add metadata for supported Python versions. diff --git a/src/cattr/disambiguators.py b/src/cattr/disambiguators.py index 2c472191..ecc2e233 100644 --- a/src/cattr/disambiguators.py +++ b/src/cattr/disambiguators.py @@ -10,7 +10,7 @@ Type, ) -from attr import fields +from attr import fields, NOTHING from cattr._compat import get_origin @@ -42,7 +42,14 @@ def create_uniq_field_dis_func(*classes: Type) -> Callable: if not uniq: m = "{} has no usable unique attributes.".format(cl) raise ValueError(m) - uniq_attrs_dict[next(iter(uniq))] = cl + # We need a unique attribute with no default. + cl_fields = fields(get_origin(cl) or cl) + for attr_name in uniq: + if getattr(cl_fields, attr_name).default is NOTHING: + break + else: + raise ValueError(f"{cl} has no usable non-default attributes.") + uniq_attrs_dict[attr_name] = cl else: fallback = cl diff --git a/tests/test_disambigutors.py b/tests/test_disambigutors.py index c9078e7b..4da1ca14 100644 --- a/tests/test_disambigutors.py +++ b/tests/test_disambigutors.py @@ -1,8 +1,11 @@ """Tests for auto-disambiguators.""" +from typing import Any + import attr import pytest -from hypothesis import assume, given +from attr import NOTHING +from hypothesis import HealthCheck, assume, given, settings from cattr.disambiguators import create_uniq_field_dis_func @@ -40,6 +43,18 @@ class D(object): # No unique fields on either class. create_uniq_field_dis_func(C, D) + @attr.s + class E: + pass + + @attr.s + class F: + b = attr.ib(default=Any) + + with pytest.raises(ValueError): + # no usable non-default attributes + create_uniq_field_dis_func(E, F) + @given(simple_classes(defaults=False)) def test_fallback(cl_and_vals): @@ -63,9 +78,12 @@ class A(object): fn({"xyz": 1}) is A # Uses the fallback. +@settings( + suppress_health_check=[HealthCheck.filter_too_much, HealthCheck.too_slow] +) @given(simple_classes(), simple_classes()) def test_disambiguation(cl_and_vals_a, cl_and_vals_b): - """Disambiguation should work when there are unique fields.""" + """Disambiguation should work when there are unique required fields.""" cl_a, vals_a = cl_and_vals_a cl_b, vals_b = cl_and_vals_b @@ -76,6 +94,10 @@ def test_disambiguation(cl_and_vals_a, cl_and_vals_b): assume(len(req_b)) assume((req_a - req_b) or (req_b - req_a)) + for attr_name in req_a - req_b: + assume(getattr(attr.fields(cl_a), attr_name).default is NOTHING) + for attr_name in req_b - req_a: + assume(getattr(attr.fields(cl_b), attr_name).default is NOTHING) fn = create_uniq_field_dis_func(cl_a, cl_b) diff --git a/tests/test_structure.py b/tests/test_structure.py index b41957cc..bbdde276 100644 --- a/tests/test_structure.py +++ b/tests/test_structure.py @@ -29,6 +29,7 @@ ) from pytest import raises +import attr from cattr import Converter from cattr._compat import is_bare, is_union_type from cattr.converters import NoneType @@ -351,3 +352,21 @@ class Bar(Foo): converter.register_structure_hook(Bar, lambda obj, cls: cls("bar")) assert converter.structure(None, Foo).value == "foo" assert converter.structure(None, Bar).value == "bar" + + +def test_structure_union_edge_case(): + converter = Converter() + + @attr.s(auto_attribs=True) + class A: + a1: Any + a2: Optional[Any] = None + + @attr.s(auto_attribs=True) + class B: + b1: Any + b2: Optional[Any] = None + + assert converter.structure( + [{"a1": "foo"}, {"b1": "bar"}], List[Union[A, B]] + ) == [A("foo"), B("bar")]