From e2fcab52b9075826c4a22cc5e8b1502b95d043b8 Mon Sep 17 00:00:00 2001 From: PrettyWood Date: Wed, 15 Jul 2020 21:23:18 +0200 Subject: [PATCH] fix: validate and parse nested models properly with `default_factory` (#1712) * fix: validate nested models with `default_factory` PR #1504 introduced a regression by bypassing `populate_validators()`, which would skip the validation of children in nested models with `default_factory` closes #1710 * test: add example of nested models parsing with `default_factory` closes #1717 * add testcase from #1722 * bodge for benchmarks Co-authored-by: Samuel Colvin --- benchmarks/run.py | 6 +++--- changes/1710-PrettyWood.md | 1 + pydantic/fields.py | 26 ++++++++++++++++---------- tests/test_edge_cases.py | 16 ++++++++++++++++ tests/test_main.py | 30 ++++++++++++++++++++++++++++++ 5 files changed, 66 insertions(+), 13 deletions(-) create mode 100644 changes/1710-PrettyWood.md diff --git a/benchmarks/run.py b/benchmarks/run.py index dcb5982..0f74d95 100644 --- a/benchmarks/run.py +++ b/benchmarks/run.py @@ -274,6 +274,6 @@ if __name__ == '__main__': else: main() - if None in other_tests: - print('not all libraries could be imported!') - sys.exit(1) + # if None in other_tests: + # print('not all libraries could be imported!') + # sys.exit(1) diff --git a/changes/1710-PrettyWood.md b/changes/1710-PrettyWood.md new file mode 100644 index 0000000..67926fe --- /dev/null +++ b/changes/1710-PrettyWood.md @@ -0,0 +1 @@ +fix validation and parsing of nested models with `default_factory` \ No newline at end of file diff --git a/pydantic/fields.py b/pydantic/fields.py index 89c70da..01792b6 100644 --- a/pydantic/fields.py +++ b/pydantic/fields.py @@ -341,8 +341,22 @@ class ModelField(Representation): e.g. calling it it multiple times may modify the field and configure it incorrectly. """ - # To prevent side effects by calling the `default_factory` for nothing, we only call it - # when we want to validate the default value i.e. when `validate_all` is set to True. + self._set_default_and_type() + self._type_analysis() + if self.required is Undefined: + self.required = True + self.field_info.default = Required + if self.default is Undefined and self.default_factory is None: + self.default = None + self.populate_validators() + + def _set_default_and_type(self) -> None: + """ + Set the default value, infer the type if needed and check if `None` value is valid. + + Note: to prevent side effects by calling the `default_factory` for nothing, we only call it + when we want to validate the default value i.e. when `validate_all` is set to True. + """ if self.default_factory is not None: if self.type_ is None: raise errors_.ConfigError( @@ -368,14 +382,6 @@ class ModelField(Representation): if self.required is False and default_value is None: self.allow_none = True - self._type_analysis() - if self.required is Undefined: - self.required = True - self.field_info.default = Required - if self.default is Undefined and self.default_factory is None: - self.default = None - self.populate_validators() - def _type_analysis(self) -> None: # noqa: C901 (ignore complexity) # typing interface is horrible, we have to do some ugly checks if lenient_issubclass(self.type_, JsonWrapper): diff --git a/tests/test_edge_cases.py b/tests/test_edge_cases.py index 36acb17..d1060c1 100644 --- a/tests/test_edge_cases.py +++ b/tests/test_edge_cases.py @@ -1714,3 +1714,19 @@ def test_default_factory_side_effect(): m1 = MyModel() assert m1.id == 2 # instead of 1 + + +def test_default_factory_validator_child(): + class Parent(BaseModel): + foo: List[str] = Field(default_factory=list) + + @validator('foo', pre=True, each_item=True) + def mutate_foo(cls, v): + return f'{v}-1' + + assert Parent(foo=['a', 'b']).foo == ['a-1', 'b-1'] + + class Child(Parent): + pass + + assert Child(foo=['a', 'b']).foo == ['a-1', 'b-1'] diff --git a/tests/test_main.py b/tests/test_main.py index 9912aa0..19d17e6 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -1174,6 +1174,36 @@ def test_default_factory_called_once_2(): assert m2.id == 2 +def test_default_factory_validate_children(): + class Child(BaseModel): + x: int + + class Parent(BaseModel): + children: List[Child] = Field(default_factory=list) + + Parent(children=[{'x': 1}, {'x': 2}]) + with pytest.raises(ValidationError) as exc_info: + Parent(children=[{'x': 1}, {'y': 2}]) + + assert exc_info.value.errors() == [ + {'loc': ('children', 1, 'x'), 'msg': 'field required', 'type': 'value_error.missing'}, + ] + + +def test_default_factory_parse(): + class Inner(BaseModel): + val: int = Field(0) + + class Outer(BaseModel): + inner_1: Inner = Field(default_factory=Inner) + inner_2: Inner = Field(Inner()) + + default = Outer().dict() + parsed = Outer.parse_obj(default) + assert parsed.dict() == {'inner_1': {'val': 0}, 'inner_2': {'val': 0}} + assert repr(parsed) == 'Outer(inner_1=Inner(val=0), inner_2=Inner(val=0))' + + @pytest.mark.skipif(sys.version_info < (3, 7), reason='field constraints are set but not enforced with python 3.6') def test_none_min_max_items(): # None default