From cb2abb10b60739c00dce7cfe1809dcebf9072925 Mon Sep 17 00:00:00 2001 From: David Sommerich Date: Sat, 18 May 2019 21:11:33 +1000 Subject: [PATCH] Fix __fields_set__ not using alias field names (#517) (#518) * Fix __fields_set__ not using alias field names (#517) fix #517 * Update HISTORY.rst * Fix single quotes * Don't need to check allow_population_by_alias * Calculate fields_set in validate_model instead * Fix formatting * Add breaking change to history * Minor fixed to validate_mode * Minor fixes to tests * Add comments for names_used and fields_set in validate_model --- HISTORY.rst | 2 ++ pydantic/dataclasses.py | 2 +- pydantic/main.py | 27 +++++++++++++-------------- tests/test_edge_cases.py | 15 +++++++++++---- tests/test_main.py | 28 ++++++++++++++++++++++++++++ 5 files changed, 55 insertions(+), 19 deletions(-) diff --git a/HISTORY.rst b/HISTORY.rst index 0fa7157..a9a02c2 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -8,6 +8,8 @@ v0.26 (unreleased) * fix to schema generation for ``IPvAnyAddress``, ``IPvAnyInterface``, ``IPvAnyNetwork`` #498 by @pilosus * fix variable length tuples support, #495 by @pilosus * fix return type hint for ``create_model``, #526 by @dmontagu +* **Breaking Change:** fix ``.dict(skip_keys=True)`` skipping values set via alias (this involves changing + ``validate_model()`` to always returns ``Tuple[Dict[str, Any], Set[str], Optional[ValidationError]]``), #517 by @sommd v0.25 (2019-05-05) .................. diff --git a/pydantic/dataclasses.py b/pydantic/dataclasses.py index 9f43940..0207489 100644 --- a/pydantic/dataclasses.py +++ b/pydantic/dataclasses.py @@ -25,7 +25,7 @@ if TYPE_CHECKING: # pragma: no cover def _pydantic_post_init(self: 'DataclassType') -> None: - d = validate_model(self.__pydantic_model__, self.__dict__, cls=self.__class__) + d = validate_model(self.__pydantic_model__, self.__dict__, cls=self.__class__)[0] object.__setattr__(self, '__dict__', d) object.__setattr__(self, '__initialised__', True) if self.__post_init_original__: diff --git a/pydantic/main.py b/pydantic/main.py index ffeb0f1..5100fe6 100644 --- a/pydantic/main.py +++ b/pydantic/main.py @@ -232,11 +232,8 @@ class BaseModel(metaclass=MetaModel): if TYPE_CHECKING: # pragma: no cover self.__values__: Dict[str, Any] = {} self.__fields_set__: 'SetStr' = set() - object.__setattr__(self, '__values__', self._process_values(data)) - if self.__config__.extra is Extra.allow: - fields_set = set(data.keys()) - else: - fields_set = data.keys() & self.__values__.keys() # type: ignore + values, fields_set, _ = validate_model(self, data) + object.__setattr__(self, '__values__', values) object.__setattr__(self, '__fields_set__', fields_set) @no_type_check @@ -433,10 +430,6 @@ class BaseModel(metaclass=MetaModel): with change_exception(DictError, TypeError, ValueError): return cls(**dict(value)) # type: ignore - def _process_values(self, input_data: Any) -> 'DictStrAny': - # (casting here is slow so use ignore) - return validate_model(self, input_data) # type: ignore - @classmethod def _get_value(cls, v: Any, by_alias: bool, skip_defaults: bool) -> Any: if isinstance(v, BaseModel): @@ -576,13 +569,16 @@ def create_model( # noqa: C901 (ignore complexity) def validate_model( # noqa: C901 (ignore complexity) model: Union[BaseModel, Type[BaseModel]], input_data: 'DictStrAny', raise_exc: bool = True, cls: 'ModelOrDc' = None -) -> Union['DictStrAny', Tuple['DictStrAny', Optional[ValidationError]]]: +) -> Tuple['DictStrAny', 'SetStr', Optional[ValidationError]]: """ validate data against a model. """ values = {} errors = [] + # input_data names, possibly alias names_used = set() + # field names, never aliases + fields_set = set() config = model.__config__ check_extra = config.extra is not Extra.ignore @@ -607,8 +603,10 @@ def validate_model( # noqa: C901 (ignore complexity) if not model.__config__.validate_all and not field.validate_always: values[name] = value continue - elif check_extra: - names_used.add(field.name if using_name else field.alias) + else: + fields_set.add(name) + if check_extra: + names_used.add(field.name if using_name else field.alias) v_, errors_ = field.validate(value, values, loc=field.alias, cls=cls or model.__class__) # type: ignore if isinstance(errors_, ErrorWrapper): @@ -621,6 +619,7 @@ def validate_model( # noqa: C901 (ignore complexity) if check_extra: extra = input_data.keys() - names_used if extra: + fields_set |= extra if config.extra is Extra.allow: for f in extra: values[f] = input_data[f] @@ -629,8 +628,8 @@ def validate_model( # noqa: C901 (ignore complexity) errors.append(ErrorWrapper(ExtraError(), loc=f, config=config)) if not raise_exc: - return values, ValidationError(errors) if errors else None + return values, fields_set, ValidationError(errors) if errors else None if errors: raise ValidationError(errors) - return values + return values, fields_set, None diff --git a/tests/test_edge_cases.py b/tests/test_edge_cases.py index fb91881..d11ae25 100644 --- a/tests/test_edge_cases.py +++ b/tests/test_edge_cases.py @@ -601,9 +601,14 @@ def test_return_errors_ok(): foo: int bar: List[int] - assert validate_model(Model, {'foo': '123', 'bar': (1, 2, 3)}) == {'foo': 123, 'bar': [1, 2, 3]} - d, e = validate_model(Model, {'foo': '123', 'bar': (1, 2, 3)}, False) + assert validate_model(Model, {'foo': '123', 'bar': (1, 2, 3)}) == ( + {'foo': 123, 'bar': [1, 2, 3]}, + {'foo', 'bar'}, + None, + ) + d, f, e = validate_model(Model, {'foo': '123', 'bar': (1, 2, 3)}, False) assert d == {'foo': 123, 'bar': [1, 2, 3]} + assert f == {'foo', 'bar'} assert e is None @@ -612,12 +617,14 @@ def test_return_errors_error(): foo: int bar: List[int] - d, e = validate_model(Model, {'foo': '123', 'bar': (1, 2, 'x')}, False) + d, f, e = validate_model(Model, {'foo': '123', 'bar': (1, 2, 'x')}, False) assert d == {'foo': 123} + assert f == {'foo', 'bar'} assert e.errors() == [{'loc': ('bar', 2), 'msg': 'value is not a valid integer', 'type': 'type_error.integer'}] - d, e = validate_model(Model, {'bar': (1, 2, 3)}, False) + d, f, e = validate_model(Model, {'bar': (1, 2, 3)}, False) assert d == {'bar': [1, 2, 3]} + assert f == {'bar'} assert e.errors() == [{'loc': ('foo',), 'msg': 'field required', 'type': 'value_error.missing'}] diff --git a/tests/test_main.py b/tests/test_main.py index 2142dcf..a165abe 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -598,6 +598,34 @@ def test_skip_defaults_recursive(): assert dict(m) == {'c': 5, 'd': 2, 'e': {'a': 0, 'b': 1}} +def test_dict_skip_defaults_populated_by_alias(): + class MyModel(BaseModel): + a: str = Schema('default', alias='alias_a') + b: str = Schema('default', alias='alias_b') + + class Config: + allow_population_by_alias = True + + m = MyModel(alias_a='a') + + assert m.dict(skip_defaults=True) == {'a': 'a'} + assert m.dict(skip_defaults=True, by_alias=True) == {'alias_a': 'a'} + + +def test_dict_skip_defaults_populated_by_alias_with_extra(): + class MyModel(BaseModel): + a: str = Schema('default', alias='alias_a') + b: str = Schema('default', alias='alias_b') + + class Config: + extra = 'allow' + + m = MyModel(alias_a='a', c='c') + + assert m.dict(skip_defaults=True) == {'a': 'a', 'c': 'c'} + assert m.dict(skip_defaults=True, by_alias=True) == {'alias_a': 'a', 'c': 'c'} + + def test_dir_fields(): class MyModel(BaseModel): attribute_a: int