Allow control of automatic identifier field addition (#432)

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

@@ -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`

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.

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)

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

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__)),

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),

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__)),
         ]
 

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: