From ac8ad186cf05c1057b681a512a5c4c2f68278d03 Mon Sep 17 00:00:00 2001 From: Subhash Bhushan Date: Fri, 7 Jun 2024 12:47:59 -0700 Subject: [PATCH] Allow control of automatic identifier field addition Entities and Aggregates in Protean are automatically augmented with an identity field (named `id`), when one has not been explicitly provided. Until now, the assumption was the abstract aggregates will not need to have identifier fields, which was incorrect. This commit allows control of automatic addition of identity field with an explicit flag named `auto_add_id_field`. With this change, even abstract aggregates can have identity fields. --- docs/guides/compose-a-domain/object-model.md | 6 ++++ docs/guides/domain-definition/aggregates.md | 9 ++++- docs_src/guides/domain-definition/003.py | 2 +- src/protean/container.py | 35 +++++++++--------- src/protean/core/aggregate.py | 1 + src/protean/core/entity.py | 3 +- src/protean/core/event_sourced_aggregate.py | 3 +- tests/test_aggregates.py | 37 +++++++++++--------- 8 files changed, 57 insertions(+), 39 deletions(-) diff --git a/docs/guides/compose-a-domain/object-model.md b/docs/guides/compose-a-domain/object-model.md index 5b7d24c4..a4693458 100644 --- a/docs/guides/compose-a-domain/object-model.md +++ b/docs/guides/compose-a-domain/object-model.md @@ -54,3 +54,9 @@ You can also pass options as parameters to the decorator: ```python hl_lines="7" {! docs_src/guides/composing-a-domain/021.py !} ``` + + +### `abstract` + + +### `auto_add_id_field` \ No newline at end of file diff --git a/docs/guides/domain-definition/aggregates.md b/docs/guides/domain-definition/aggregates.md index 098f084d..ca83daf3 100644 --- a/docs/guides/domain-definition/aggregates.md +++ b/docs/guides/domain-definition/aggregates.md @@ -114,7 +114,7 @@ Available options are: ### `abstract` -When specified marks an Aggregate as abstract. If abstract, the aggregate +Marks an Aggregate as abstract if `True`. If abstract, the aggregate cannot be instantiated and needs to be subclassed. ```python hl_lines="12" @@ -130,6 +130,13 @@ NotSupportedError Traceback (most recent call last) NotSupportedError: TimeStamped class has been marked abstract and cannot be instantiated ``` +### `auto_add_id_field` + +If `True`, the aggregate will not contain an identifier field (acting as +primary key) added by default. This option is usually combined with +`abstract` to create classes that are meant to be subclassed by other +aggregates. + ### `provider` Specifies the database that the aggregate is persisted in. diff --git a/docs_src/guides/domain-definition/003.py b/docs_src/guides/domain-definition/003.py index 2900e655..a4afa4af 100644 --- a/docs_src/guides/domain-definition/003.py +++ b/docs_src/guides/domain-definition/003.py @@ -10,7 +10,7 @@ def utc_now(): return datetime.now(timezone.utc) -@domain.aggregate(abstract=True) +@domain.aggregate(abstract=True, auto_add_id_field=False) class TimeStamped: created_at = DateTime(default=utc_now) updated_at = DateTime(default=utc_now) diff --git a/src/protean/container.py b/src/protean/container.py index 920d0cb3..89a2bcdf 100644 --- a/src/protean/container.py +++ b/src/protean/container.py @@ -8,7 +8,6 @@ from typing import Any, Type, Union from protean.exceptions import ( - IncorrectUsageError, InvalidDataError, NotSupportedError, ValidationError, @@ -89,8 +88,6 @@ def __init_subclass__(subclass) -> None: Args: subclass (Protean Element): Subclass to initialize with metadata """ - super().__init_subclass__() - # Retrieve inner Meta class # Gather `Meta` class/object if defined options = getattr(subclass, "Meta", None) @@ -110,13 +107,19 @@ def __init_subclass__(subclass) -> None: # Assign default options for remaining items subclass._set_defaults() + super().__init_subclass__() + @classmethod def _set_defaults(cls): # Assign default options for remaining items # with the help of `_default_options()` method defined in the Element's Root. # Element Roots are `Event`, `Subscriber`, `Repository`, and so on. for key, default in cls._default_options(): - value = (hasattr(cls.meta_, key) and getattr(cls.meta_, key)) or default + # FIXME Should the `None` check be replaced with a SENTINEL check? + if hasattr(cls.meta_, key) and getattr(cls.meta_, key) is not None: + value = getattr(cls.meta_, key) + else: + value = default setattr(cls.meta_, key, value) def __init__(self, *args, **kwargs): @@ -361,7 +364,13 @@ class IdentityMixin: def __init_subclass__(subclass) -> None: super().__init_subclass__() - subclass.__set_id_field() + # FIXME Is there a better way to check this? + if subclass.__name__ not in [ + "BaseAggregate", + "BaseEntity", + "BaseEventSourcedAggregate", + ]: + subclass.__set_id_field() @classmethod def __set_id_field(new_class): @@ -376,23 +385,11 @@ def __set_id_field(new_class): ) setattr(new_class, _ID_FIELD_NAME, id_field.field_name) - - # If the aggregate/entity has been marked abstract, - # and contains an identifier field, raise exception - if new_class.meta_.abstract and id_field: - raise IncorrectUsageError( - { - "_entity": [ - f"Abstract Aggregate `{new_class.__name__}` marked as abstract cannot have" - " identity fields" - ] - } - ) except StopIteration: # If no id field is declared then create one - # If the aggregate/entity is marked abstract, + # If entity is explicitly marked with `auto_add_id_field=False`, # avoid creating an identifier field. - if not new_class.meta_.abstract: + if new_class.meta_.auto_add_id_field: new_class.__create_id_field() @classmethod diff --git a/src/protean/core/aggregate.py b/src/protean/core/aggregate.py index c1f8a2c6..7bdebfdc 100644 --- a/src/protean/core/aggregate.py +++ b/src/protean/core/aggregate.py @@ -58,6 +58,7 @@ def __init__(self, *args, **kwargs): @classmethod def _default_options(cls): return [ + ("auto_add_id_field", True), ("provider", "default"), ("model", None), ("stream_name", inflection.underscore(cls.__name__)), diff --git a/src/protean/core/entity.py b/src/protean/core/entity.py index 98214cc4..2b3f71b8 100644 --- a/src/protean/core/entity.py +++ b/src/protean/core/entity.py @@ -77,7 +77,7 @@ def mark_destroyed(self): fields_cache = _FieldsCacheDescriptor() -class BaseEntity(IdentityMixin, OptionsMixin, BaseContainer): +class BaseEntity(OptionsMixin, IdentityMixin, BaseContainer): """The Base class for Protean-Compliant Domain Entities. Provides helper methods to custom define entity attributes, and query attribute names @@ -122,6 +122,7 @@ def __init_subclass__(subclass) -> None: @classmethod def _default_options(cls): return [ + ("auto_add_id_field", True), ("provider", "default"), ("model", None), ("part_of", None), diff --git a/src/protean/core/event_sourced_aggregate.py b/src/protean/core/event_sourced_aggregate.py index dcbee2e3..d4f03d6d 100644 --- a/src/protean/core/event_sourced_aggregate.py +++ b/src/protean/core/event_sourced_aggregate.py @@ -22,7 +22,7 @@ class BaseEventSourcedAggregate( - IdentityMixin, EventedMixin, OptionsMixin, BaseContainer + OptionsMixin, IdentityMixin, EventedMixin, BaseContainer ): """Base Event Sourced Aggregate class that all EventSourced Aggregates should inherit from. @@ -40,6 +40,7 @@ class Meta: @classmethod def _default_options(cls): return [ + ("auto_add_id_field", True), ("stream_name", inflection.underscore(cls.__name__)), ] diff --git a/tests/test_aggregates.py b/tests/test_aggregates.py index 0fd53fda..9b51a25c 100644 --- a/tests/test_aggregates.py +++ b/tests/test_aggregates.py @@ -3,7 +3,7 @@ import pytest from protean import BaseAggregate -from protean.exceptions import IncorrectUsageError, ValidationError +from protean.exceptions import ValidationError from protean.fields import Date, DateTime, HasMany, Reference, String from protean.reflection import declared_fields from protean.utils import fully_qualified_name, utcnow_func @@ -56,7 +56,7 @@ class Person: email = String(identifier=True) username = String(identifier=True) - def test_that_abstract_aggregates_do_not_have_id_field(self, test_domain): + def test_that_abstract_aggregates_get_an_id_field_by_default(self, test_domain): @test_domain.aggregate class TimeStamped: created_at = DateTime(default=utcnow_func) @@ -65,26 +65,31 @@ class TimeStamped: class Meta: abstract = True - assert "id" not in declared_fields(TimeStamped) + assert "id" in declared_fields(TimeStamped) - def test_that_abstract_aggregates_cannot_have_a_declared_id_field( + def test_that_an_aggregate_can_opt_to_have_no_id_field_by_default( self, test_domain ): - with pytest.raises(IncorrectUsageError) as exception: + @test_domain.aggregate + class TimeStamped: + created_at = DateTime(default=utcnow_func) + updated_at = DateTime(default=utcnow_func) - @test_domain.aggregate - class User(BaseAggregate): - email = String(identifier=True) - name = String(max_length=55) + class Meta: + auto_add_id_field = False - class Meta: - abstract = True + assert "id" not in declared_fields(TimeStamped) + + def test_that_abstract_aggregates_can_have_an_explicit_id_field(self, test_domain): + @test_domain.aggregate + class User(BaseAggregate): + email = String(identifier=True) + name = String(max_length=55) + + class Meta: + abstract = True - assert exception.value.messages == { - "_entity": [ - "Abstract Aggregate `User` marked as abstract cannot have identity fields" - ] - } + assert "email" in declared_fields(User) class TestAggregateMeta: