fix(field): remove some side effects of default_factory (#1504)

* Avoid some side effects of default factory

- by calling it only once if possible (fix #1491)
- by not setting the default value in the schema (fix #1520)

* refactor: ensure type is set when using default_factory

changes/1491-PrettyWood.md

@@ -0,0 +1,2 @@
+Avoid some side effects of `default_factory` by calling it only once
+if possible and by not setting a default value in the schema

docs/usage/models.md

@@ -512,6 +512,11 @@ _(This script is complete, it should run "as is")_
 
 Where `Field` refers to the [field function](schema.md#field-customisation).
 
+!!! warning
+    The `default_factory` expects the field type to be set.
+    Moreover if you want to validate default values with `validate_all`,
+    *pydantic* will need to call the `default_factory`, which could lead to side effects!
+
 ## Parsing data into a specified type
 
 Pydantic includes a standalone utility function `parse_obj_as` that can be used to apply the parsing

pydantic/fields.py

@@ -295,7 +295,7 @@ def infer(
 
         if isinstance(value, FieldInfo):
             field_info = value
-            value = field_info.default_factory() if field_info.default_factory is not None else field_info.default
+            value = None if field_info.default_factory is not None else field_info.default
         else:
             field_info = FieldInfo(value, **field_info_from_config)
         required: 'BoolUndefined' = Undefined
@@ -340,7 +340,19 @@ def prepare(self) -> None:
         Note: this method is **not** idempotent (because _type_analysis is not idempotent),
         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.
+        if self.default_factory is not None:
+            if self.type_ is None:
+                raise errors_.ConfigError(
+                    f'you need to set the type of field {self.name!r} when using `default_factory`'
+                )
+            if not self.model_config.validate_all:
+                return
+
         default_value = self.get_default()
+
         if default_value is not None and self.type_ is None:
             self.type_ = default_value.__class__
             self.outer_type_ = self.type_

tests/test_edge_cases.py

@@ -1531,3 +1531,23 @@ class Model(BaseModel):
 
     Model(v=None)
     Model()
+
+
+def test_default_factory_side_effect():
+    """It may call `default_factory` more than once when `validate_all` is set"""
+
+    v = 0
+
+    def factory():
+        nonlocal v
+        v += 1
+        return v
+
+    class MyModel(BaseModel):
+        id: int = Field(default_factory=factory)
+
+        class Config:
+            validate_all = True
+
+    m1 = MyModel()
+    assert m1.id == 2  # instead of 1

tests/test_main.py

@@ -5,7 +5,7 @@
 
 import pytest
 
-from pydantic import BaseModel, Extra, Field, NoneBytes, NoneStr, Required, ValidationError, constr
+from pydantic import BaseModel, ConfigError, Extra, Field, NoneBytes, NoneStr, Required, ValidationError, constr
 
 
 def test_success():
@@ -52,26 +52,32 @@ def test_ultra_simple_repr():
         assert m.to_string() == 'a=10.2 b=10'
 
 
-def test_default_dict_repr():
+def test_default_factory_field():
     def myfunc():
         return 1
 
     class Model(BaseModel):
         a: int = Field(default_factory=myfunc)
-        b = Field(default_factory=myfunc)
 
     m = Model()
-    assert str(m) == 'a=1 b=1'
-    assert repr(m) == 'Model(a=1, b=1)'
+    assert str(m) == 'a=1'
     assert (
         repr(m.__fields__['a']) == "ModelField(name='a', type=int, required=False, default_factory='<function myfunc>')"
     )
-    assert (
-        repr(m.__fields__['b']) == "ModelField(name='b', type=int, required=False, default_factory='<function myfunc>')"
-    )
-    assert dict(m) == {'a': 1, 'b': 1}
-    assert m.dict() == {'a': 1, 'b': 1}
-    assert m.json() == '{"a": 1, "b": 1}'
+    assert dict(m) == {'a': 1}
+    assert m.json() == '{"a": 1}'
+
+
+def test_default_factory_no_type_field():
+    def myfunc():
+        return 1
+
+    with pytest.raises(ConfigError) as e:
+
+        class Model(BaseModel):
+            a = Field(default_factory=myfunc)
+
+    assert str(e.value) == "you need to set the type of field 'a' when using `default_factory`"
 
 
 def test_comparing():
@@ -1102,6 +1108,46 @@ class Config:
     assert SingletonFieldModel().singleton is SingletonFieldModel().singleton
 
 
+def test_default_factory_called_once():
+    """It should call only once the given factory by default"""
+
+    class Seq:
+        def __init__(self):
+            self.v = 0
+
+        def __call__(self):
+            self.v += 1
+            return self.v
+
+    class MyModel(BaseModel):
+        id: int = Field(default_factory=Seq())
+
+    m1 = MyModel()
+    assert m1.id == 1
+    m2 = MyModel()
+    assert m2.id == 2
+    assert m1.id == 1
+
+
+def test_default_factory_called_once_2():
+    """It should call only once the given factory by default"""
+
+    v = 0
+
+    def factory():
+        nonlocal v
+        v += 1
+        return v
+
+    class MyModel(BaseModel):
+        id: int = Field(default_factory=factory)
+
+    m1 = MyModel()
+    assert m1.id == 1
+    m2 = MyModel()
+    assert m2.id == 2
+
+
 @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