Disallow multiple identity fields in aggregates

Protean can only work with a single identity field for now. It is not clear
if composite keys are good from a DDD perspective.

src/protean/container.py

@@ -12,7 +12,7 @@
     NotSupportedError,
     ValidationError,
 )
-from protean.fields import Auto, Field, FieldBase, Reference, ValueObject
+from protean.fields import Auto, Field, FieldBase, ValueObject
 from protean.utils import generate_identity
 
 from .reflection import _FIELDS, _ID_FIELD_NAME, attributes, declared_fields, fields
@@ -365,15 +365,27 @@ def __set_id_field(new_class):
         """Lookup the id field for this entity and assign"""
         # FIXME What does it mean when there are no declared fields?
         #   Does it translate to an abstract entity?
-        try:
-            id_field = next(
-                field
-                for _, field in declared_fields(new_class).items()
-                if isinstance(field, (Field, Reference)) and field.identifier
+        id_fields = [
+            field
+            for _, field in declared_fields(new_class).items()
+            if isinstance(field, Field) and field.identifier
+        ]
+
+        if len(id_fields) > 1:
+            raise NotSupportedError(
+                {
+                    "_entity": [
+                        f"Multiple identifier fields found in entity {new_class.__name__}. "
+                        f"Only one identifier field is allowed."
+                    ]
+                }
             )
 
-            setattr(new_class, _ID_FIELD_NAME, id_field.field_name)
-        except StopIteration:
+        elif len(id_fields) == 1:
+            # Remember the identity field
+            setattr(new_class, _ID_FIELD_NAME, id_fields[0].field_name)
+
+        else:
             # If no id field is declared then create one
             #   If entity is explicitly marked with `auto_add_id_field=False`,
             #   avoid creating an identifier field.

src/protean/core/event_sourced_aggregate.py

@@ -9,8 +9,8 @@
 from protean.container import BaseContainer, EventedMixin, IdentityMixin, OptionsMixin
 from protean.core.event import BaseEvent
 from protean.exceptions import IncorrectUsageError, NotSupportedError
-from protean.fields import Field, Integer
-from protean.reflection import _ID_FIELD_NAME, declared_fields, has_fields, id_field
+from protean.fields import Integer
+from protean.reflection import id_field
 from protean.utils import (
     DomainObjects,
     derive_element_class,
@@ -49,9 +49,6 @@ def _default_options(cls):
     def __init_subclass__(subclass) -> None:
         super().__init_subclass__()
 
-        if not subclass.meta_.abstract:
-            subclass.__validate_id_field()
-
         # Associate a `_projections` map with subclasses.
         #   It needs to be initialized here because if it
         #   were initialized in __init__, the same collection object
@@ -62,30 +59,6 @@ def __init_subclass__(subclass) -> None:
         # Store associated events
         setattr(subclass, "_events_cls_map", {})
 
-    @classmethod
-    def __validate_id_field(subclass):
-        """Lookup the id field for this view and assign"""
-        # FIXME What does it mean when there are no declared fields?
-        #   Does it translate to an abstract view?
-        if has_fields(subclass):
-            try:
-                id_field = next(
-                    field
-                    for _, field in declared_fields(subclass).items()
-                    if isinstance(field, (Field)) and field.identifier
-                )
-
-                setattr(subclass, _ID_FIELD_NAME, id_field.field_name)
-
-            except StopIteration:
-                raise IncorrectUsageError(
-                    {
-                        "_entity": [
-                            f"Event Sourced Aggregate `{subclass.__name__}` needs to have at least one identifier"
-                        ]
-                    }
-                )
-
     def __eq__(self, other):
         """Equivalence check to be based only on Identity"""
 

tests/event_sourced_aggregates/test_auto_id_field.py

@@ -1,6 +1,7 @@
 from protean import BaseEventSourcedAggregate
 from protean.fields import Auto, DateTime, Identifier, Integer, String
-from protean.reflection import fields, id_field
+from protean.reflection import fields, id_field, declared_fields
+from protean.utils import utcnow_func
 
 
 class User(BaseEventSourcedAggregate):
@@ -27,3 +28,12 @@ def test_no_auto_id_field_generation_when_an_identifier_is_provided():
 
     field_obj = id_field(Order)
     assert field_obj.field_name == "order_id"
+
+
+def test_that_an_aggregate_can_opt_to_have_no_id_field_by_default(test_domain):
+    @test_domain.event_sourced_aggregate(auto_add_id_field=False)
+    class TimeStamped:
+        created_at = DateTime(default=utcnow_func)
+        updated_at = DateTime(default=utcnow_func)
+
+    assert "id" not in declared_fields(TimeStamped)

tests/event_sourced_aggregates/test_validations.py

@@ -0,0 +1,15 @@
+import pytest
+
+from protean.exceptions import NotSupportedError
+from protean.fields import String
+
+
+def test_exception_on_multiple_identifiers(test_domain):
+    with pytest.raises(NotSupportedError) as exc:
+
+        @test_domain.event_sourced_aggregate
+        class Person:
+            email = String(identifier=True)
+            username = String(identifier=True)
+
+    assert "Only one identifier field is allowed" in exc.value.args[0]["_entity"][0]

tests/test_aggregates.py

@@ -3,7 +3,7 @@
 import pytest
 
 from protean import BaseAggregate
-from protean.exceptions import ValidationError
+from protean.exceptions import ValidationError, NotSupportedError
 from protean.fields import Date, DateTime, HasMany, Reference, String
 from protean.reflection import declared_fields
 from protean.utils import fully_qualified_name, utcnow_func
@@ -49,12 +49,15 @@ class TestAggregateInitialization:
 
 
 class TestAggregateIdentity:
-    # FIXME This should fail
     def test_exception_on_multiple_identifiers(self, test_domain):
-        @test_domain.aggregate
-        class Person:
-            email = String(identifier=True)
-            username = String(identifier=True)
+        with pytest.raises(NotSupportedError) as exc:
+
+            @test_domain.aggregate
+            class Person:
+                email = String(identifier=True)
+                username = String(identifier=True)
+
+        assert "Only one identifier field is allowed" in exc.value.args[0]["_entity"][0]
 
     def test_that_abstract_aggregates_get_an_id_field_by_default(self, test_domain):
         @test_domain.aggregate(abstract=True)