From 18e422bc2d6f87c5799bdbae0e1e00a3b089f5e6 Mon Sep 17 00:00:00 2001 From: Subhash Bhushan Date: Wed, 5 Jun 2024 18:00:06 -0700 Subject: [PATCH] Run `Entity.defaults` before validations The `defaults` method was being run after setting `None` values to remaining fields which would already triggered errors for fields marked `required`. This commit runs `defaults` before so that field values can be set in `defaults` method safely before firing ValidationError exceptions. This method also contains fixes for test_support domains that need to explicitly look for a domain.toml config file. --- src/protean/core/entity.py | 8 +++++--- tests/adapters/model/dict_model/tests.py | 6 +++--- tests/entity/test_lifecycle_methods.py | 11 +++++++++++ tests/support/test_domains/test18/domain18.py | 2 +- tests/support/test_domains/test19/domain19.py | 2 +- 5 files changed, 21 insertions(+), 8 deletions(-) diff --git a/src/protean/core/entity.py b/src/protean/core/entity.py index 923cfc3f..98214cc4 100644 --- a/src/protean/core/entity.py +++ b/src/protean/core/entity.py @@ -306,10 +306,14 @@ def __init__(self, *template, **kwargs): # noqa: C901 self, f"filter_{field_name}", partial(field_obj.filter, self) ) + self.defaults() + # Now load the remaining fields with a None value, which will fail # for required fields for field_name, field_obj in fields(self).items(): - if field_name not in loaded_fields: + if field_name not in loaded_fields and ( + not hasattr(self, field_name) or getattr(self, field_name) is None + ): if not isinstance(field_obj, Association): try: setattr(self, field_name, None) @@ -326,8 +330,6 @@ def __init__(self, *template, **kwargs): # noqa: C901 if field_name not in loaded_fields and not hasattr(self, field_name): setattr(self, field_name, None) - self.defaults() - self._initialized = True # `_postcheck()` will return a `defaultdict(list)` if errors are to be raised diff --git a/tests/adapters/model/dict_model/tests.py b/tests/adapters/model/dict_model/tests.py index feca25ca..20402663 100644 --- a/tests/adapters/model/dict_model/tests.py +++ b/tests/adapters/model/dict_model/tests.py @@ -15,7 +15,7 @@ def test_that_model_class_is_created_automatically(self, test_domain): assert issubclass(model_cls, MemoryModel) assert model_cls.__name__ == "PersonModel" - def test_conversation_from_entity_to_model(self, test_domain): + def test_conversion_from_entity_to_model(self, test_domain): model_cls = test_domain.repository_for(Person)._model person = Person(first_name="John", last_name="Doe") @@ -28,7 +28,7 @@ def test_conversation_from_entity_to_model(self, test_domain): assert person_model_obj["first_name"] == "John" assert person_model_obj["last_name"] == "Doe" - def test_conversation_from_model_to_entity(self, test_domain): + def test_conversion_from_model_to_entity(self, test_domain): model_cls = test_domain.repository_for(Person)._model person = Person(first_name="John", last_name="Doe") person_model_obj = model_cls.from_entity(person) @@ -47,7 +47,7 @@ def test_that_model_class_is_created_automatically(self, test_domain): assert issubclass(model_cls, MemoryModel) assert model_cls.__name__ == "UserModel" - def test_conversation_from_entity_to_model(self, test_domain): + def test_conversion_from_entity_to_model(self, test_domain): model_cls = test_domain.repository_for(User)._model user1 = User(email_address="john.doe@gmail.com", password="d4e5r6") diff --git a/tests/entity/test_lifecycle_methods.py b/tests/entity/test_lifecycle_methods.py index ed4bbd3a..aff57514 100644 --- a/tests/entity/test_lifecycle_methods.py +++ b/tests/entity/test_lifecycle_methods.py @@ -1,5 +1,7 @@ import pytest +from protean import BaseEntity +from protean.fields import String from protean.exceptions import ValidationError from .elements import Area, Building, BuildingStatus @@ -16,6 +18,15 @@ def test_that_building_is_marked_as_done_if_below_4_floors(self): assert building.status == BuildingStatus.WIP.value + def test_defaults_are_applied_before_validation(self): + class Foo(BaseEntity): + name = String(required=True) + + def defaults(self): + self.name = "bar" + + assert Foo().name == "bar" + class TestInvariantValidation: def test_that_building_cannot_be_WIP_if_above_4_floors(self, test_domain): diff --git a/tests/support/test_domains/test18/domain18.py b/tests/support/test_domains/test18/domain18.py index 0e40f614..16c02e34 100644 --- a/tests/support/test_domains/test18/domain18.py +++ b/tests/support/test_domains/test18/domain18.py @@ -1,4 +1,4 @@ from protean import Domain -domain = Domain(__file__, "TEST18", load_toml=False) +domain = Domain(__file__, "TEST18") diff --git a/tests/support/test_domains/test19/domain19.py b/tests/support/test_domains/test19/domain19.py index a6c7d59c..bec12c47 100644 --- a/tests/support/test_domains/test19/domain19.py +++ b/tests/support/test_domains/test19/domain19.py @@ -1,4 +1,4 @@ from protean import Domain -domain = Domain(__file__, "TEST19", load_toml=False) +domain = Domain(__file__, "TEST19")