Skip to content

Commit e2fcab5

Browse files
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 <s@muelcolvin.com>
1 parent ba56a67 commit e2fcab5

File tree

5 files changed

+66
-13
lines changed

5 files changed

+66
-13
lines changed

benchmarks/run.py

+3-3
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,6 @@ def diff():
274274
else:
275275
main()
276276

277-
if None in other_tests:
278-
print('not all libraries could be imported!')
279-
sys.exit(1)
277+
# if None in other_tests:
278+
# print('not all libraries could be imported!')
279+
# sys.exit(1)

changes/1710-PrettyWood.md

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
fix validation and parsing of nested models with `default_factory`

pydantic/fields.py

+16-10
Original file line numberDiff line numberDiff line change
@@ -341,8 +341,22 @@ def prepare(self) -> None:
341341
e.g. calling it it multiple times may modify the field and configure it incorrectly.
342342
"""
343343

344-
# To prevent side effects by calling the `default_factory` for nothing, we only call it
345-
# when we want to validate the default value i.e. when `validate_all` is set to True.
344+
self._set_default_and_type()
345+
self._type_analysis()
346+
if self.required is Undefined:
347+
self.required = True
348+
self.field_info.default = Required
349+
if self.default is Undefined and self.default_factory is None:
350+
self.default = None
351+
self.populate_validators()
352+
353+
def _set_default_and_type(self) -> None:
354+
"""
355+
Set the default value, infer the type if needed and check if `None` value is valid.
356+
357+
Note: to prevent side effects by calling the `default_factory` for nothing, we only call it
358+
when we want to validate the default value i.e. when `validate_all` is set to True.
359+
"""
346360
if self.default_factory is not None:
347361
if self.type_ is None:
348362
raise errors_.ConfigError(
@@ -368,14 +382,6 @@ def prepare(self) -> None:
368382
if self.required is False and default_value is None:
369383
self.allow_none = True
370384

371-
self._type_analysis()
372-
if self.required is Undefined:
373-
self.required = True
374-
self.field_info.default = Required
375-
if self.default is Undefined and self.default_factory is None:
376-
self.default = None
377-
self.populate_validators()
378-
379385
def _type_analysis(self) -> None: # noqa: C901 (ignore complexity)
380386
# typing interface is horrible, we have to do some ugly checks
381387
if lenient_issubclass(self.type_, JsonWrapper):

tests/test_edge_cases.py

+16
Original file line numberDiff line numberDiff line change
@@ -1714,3 +1714,19 @@ class Config:
17141714

17151715
m1 = MyModel()
17161716
assert m1.id == 2 # instead of 1
1717+
1718+
1719+
def test_default_factory_validator_child():
1720+
class Parent(BaseModel):
1721+
foo: List[str] = Field(default_factory=list)
1722+
1723+
@validator('foo', pre=True, each_item=True)
1724+
def mutate_foo(cls, v):
1725+
return f'{v}-1'
1726+
1727+
assert Parent(foo=['a', 'b']).foo == ['a-1', 'b-1']
1728+
1729+
class Child(Parent):
1730+
pass
1731+
1732+
assert Child(foo=['a', 'b']).foo == ['a-1', 'b-1']

tests/test_main.py

+30
Original file line numberDiff line numberDiff line change
@@ -1174,6 +1174,36 @@ class MyModel(BaseModel):
11741174
assert m2.id == 2
11751175

11761176

1177+
def test_default_factory_validate_children():
1178+
class Child(BaseModel):
1179+
x: int
1180+
1181+
class Parent(BaseModel):
1182+
children: List[Child] = Field(default_factory=list)
1183+
1184+
Parent(children=[{'x': 1}, {'x': 2}])
1185+
with pytest.raises(ValidationError) as exc_info:
1186+
Parent(children=[{'x': 1}, {'y': 2}])
1187+
1188+
assert exc_info.value.errors() == [
1189+
{'loc': ('children', 1, 'x'), 'msg': 'field required', 'type': 'value_error.missing'},
1190+
]
1191+
1192+
1193+
def test_default_factory_parse():
1194+
class Inner(BaseModel):
1195+
val: int = Field(0)
1196+
1197+
class Outer(BaseModel):
1198+
inner_1: Inner = Field(default_factory=Inner)
1199+
inner_2: Inner = Field(Inner())
1200+
1201+
default = Outer().dict()
1202+
parsed = Outer.parse_obj(default)
1203+
assert parsed.dict() == {'inner_1': {'val': 0}, 'inner_2': {'val': 0}}
1204+
assert repr(parsed) == 'Outer(inner_1=Inner(val=0), inner_2=Inner(val=0))'
1205+
1206+
11771207
@pytest.mark.skipif(sys.version_info < (3, 7), reason='field constraints are set but not enforced with python 3.6')
11781208
def test_none_min_max_items():
11791209
# None default

0 commit comments

Comments
 (0)