Miscellaneous Bugfixes

- Do not allow associations and references in Events and Commands
- Use camelcase domain name when constructing event/command type
- Validate ValueObject field is connected to a value object
- Refactor repository methods to indicate support for both aggregates and views

src/protean/core/command.py

@@ -7,6 +7,7 @@
     ValidationError,
 )
 from protean.fields import Field, ValueObject
+from protean.fields.association import Association, Reference
 from protean.globals import g
 from protean.reflection import _ID_FIELD_NAME, declared_fields, fields
 from protean.utils import DomainObjects, derive_element_class, fqn
@@ -39,6 +40,22 @@ def __init_subclass__(subclass) -> None:
         if not hasattr(subclass, "__version__"):
             setattr(subclass, "__version__", "v1")
 
+        subclass.__validate_for_basic_field_types()
+
+    @classmethod
+    def __validate_for_basic_field_types(subclass):
+        for field_name, field_obj in fields(subclass).items():
+            # Value objects can hold all kinds of fields, except associations
+            if isinstance(field_obj, (Reference, Association)):
+                raise IncorrectUsageError(
+                    {
+                        "_event": [
+                            f"Commands cannot have associations. "
+                            f"Remove {field_name} ({field_obj.__class__.__name__}) from class {subclass.__name__}"
+                        ]
+                    }
+                )
+
     def __init__(self, *args, **kwargs):
         try:
             super().__init__(*args, finalize=False, **kwargs)

src/protean/core/event.py

@@ -10,6 +10,7 @@
     NotSupportedError,
 )
 from protean.fields import DateTime, Field, Integer, String, ValueObject
+from protean.fields.association import Association, Reference
 from protean.globals import g
 from protean.reflection import _ID_FIELD_NAME, declared_fields, fields
 from protean.utils import DomainObjects, derive_element_class, fqn
@@ -90,6 +91,22 @@ def __init_subclass__(subclass) -> None:
         if not hasattr(subclass, "__version__"):
             setattr(subclass, "__version__", "v1")
 
+        subclass.__validate_for_basic_field_types()
+
+    @classmethod
+    def __validate_for_basic_field_types(subclass):
+        for field_name, field_obj in fields(subclass).items():
+            # Value objects can hold all kinds of fields, except associations
+            if isinstance(field_obj, (Reference, Association)):
+                raise IncorrectUsageError(
+                    {
+                        "_event": [
+                            f"Events cannot have associations. "
+                            f"Remove {field_name} ({field_obj.__class__.__name__}) from class {subclass.__name__}"
+                        ]
+                    }
+                )
+
     def __setattr__(self, name, value):
         if not hasattr(self, "_initialized") or not self._initialized:
             return super().__setattr__(name, value)

src/protean/core/repository.py

@@ -2,11 +2,12 @@
 
 import logging
 from functools import lru_cache
-from typing import TYPE_CHECKING
+from typing import TYPE_CHECKING, Union
 
 from protean.container import Element, OptionsMixin
 from protean.core.aggregate import BaseAggregate
 from protean.core.unit_of_work import UnitOfWork
+from protean.core.view import BaseView
 from protean.exceptions import IncorrectUsageError, NotSupportedError
 from protean.fields import HasMany, HasOne
 from protean.globals import current_domain, current_uow
@@ -95,10 +96,12 @@ def _dao(self) -> BaseDAO:
         # Fixate on Model class at the domain level because an explicit model may have been registered
         return self._provider.get_dao(self.meta_.part_of, self._model)
 
-    def add(self, aggregate: BaseAggregate) -> BaseAggregate:  # noqa: C901
-        """This method helps persist or update aggregates into the persistence store.
+    def add(
+        self, item: Union[BaseAggregate, BaseView]
+    ) -> Union[BaseAggregate, BaseView]:  # noqa: C901
+        """This method helps persist or update aggregates or views into the persistence store.
 
-        Returns the persisted aggregate.
+        Returns the persisted item.
 
         Protean adopts a collection-oriented design pattern to handle persistence. What this means is that
         the Repository interface does not hint in any way that there is an underlying persistence mechanism,
@@ -124,31 +127,31 @@ def add(self, aggregate: BaseAggregate) -> BaseAggregate:  # noqa: C901
             own_current_uow.start()
 
         # If there are HasMany/HasOne fields in the aggregate, sync child objects added/removed,
-        if has_association_fields(aggregate):
-            self._sync_children(aggregate)
+        if has_association_fields(item):
+            self._sync_children(item)
 
-        # Persist only if the aggregate object is new, or it has changed since last persistence
-        if (not aggregate.state_.is_persisted) or (
-            aggregate.state_.is_persisted and aggregate.state_.is_changed
+        # Persist only if the item object is new, or it has changed since last persistence
+        if (not item.state_.is_persisted) or (
+            item.state_.is_persisted and item.state_.is_changed
         ):
-            self._dao.save(aggregate)
+            self._dao.save(item)
 
             # If Aggregate has signed up Fact Events, raise them now
-            if aggregate.meta_.fact_events:
-                payload = aggregate.to_dict()
+            if item.element_type == DomainObjects.AGGREGATE and item.meta_.fact_events:
+                payload = item.to_dict()
 
                 # Remove state attribute from the payload, as it is not needed for the Fact Event
                 payload.pop("state_", None)
 
                 # Construct and raise the Fact Event
-                fact_event = aggregate._fact_event_cls(**payload)
-                aggregate.raise_(fact_event)
+                fact_event = item._fact_event_cls(**payload)
+                item.raise_(fact_event)
 
         # If we started a UnitOfWork, commit it now
         if own_current_uow:
             own_current_uow.commit()
 
-        return aggregate
+        return item
 
     def _sync_children(self, entity):
         """Recursively sync child entities to the persistence store"""
@@ -232,11 +235,11 @@ def _sync_children(self, entity):
                     entity._temp_cache[field_name]["change"] = None
                     entity._temp_cache[field_name]["old_value"] = None
 
-    def get(self, identifier) -> BaseAggregate:
+    def get(self, identifier) -> Union[BaseAggregate, BaseView]:
         """This is a utility method to fetch data from the persistence store by its key identifier. All child objects,
         including enclosed entities, are returned as part of this call.
 
-        Returns the fetched aggregate.
+        Returns the fetched object.
 
         All other data filtering capabilities can be implemented by using the underlying DAO's
         :meth:`BaseDAO.filter` method.
@@ -245,16 +248,17 @@ def get(self, identifier) -> BaseAggregate:
         `find_residents_of_area(zipcode)`, etc. It is also possible to make use of more complicated,
         domain-friendly design patterns like the `Specification` pattern.
         """
-        aggregate = self._dao.get(identifier)
+        item = self._dao.get(identifier)
 
-        # Fetch and sync events version
-        last_message = current_domain.event_store.store.read_last_message(
-            f"{aggregate.meta_.stream_category}-{identifier}"
-        )
-        if last_message:
-            aggregate._event_position = last_message.position
+        if item.element_type == DomainObjects.AGGREGATE:
+            # Fetch and sync events version
+            last_message = current_domain.event_store.store.read_last_message(
+                f"{item.meta_.stream_category}-{identifier}"
+            )
+            if last_message:
+                item._event_position = last_message.position
 
-        return aggregate
+        return item
 
 
 def repository_factory(element_cls, domain, **opts):

src/protean/domain/__init__.py

@@ -834,7 +834,7 @@ def _set_event_and_command_type(self):
                     element.cls,
                     "__type__",
                     (
-                        f"{self.name}."
+                        f"{self.camel_case_name}."
                         # f"{element.cls.meta_.aggregate_cluster.__class__.__name__}."
                         f"{element.cls.__name__}."
                         f"{element.cls.__version__}"

src/protean/fields/embedded.py

@@ -2,6 +2,7 @@
 
 from functools import lru_cache
 
+from protean.exceptions import IncorrectUsageError
 from protean.fields import Field
 from protean.reflection import declared_fields
 
@@ -57,16 +58,37 @@ class ValueObject(Field):
 
     def __init__(self, value_object_cls, *args, **kwargs):
         super().__init__(*args, **kwargs)
+
+        if not isinstance(value_object_cls, str):
+            # Validate the class being passed is a subclass of BaseValueObject
+            self._validate_value_object_cls(value_object_cls)
+
         self._value_object_cls = value_object_cls
 
         self._embedded_fields = {}
 
+    def _validate_value_object_cls(self, value_object_cls):
+        """Validate that the value object class is a subclass of BaseValueObject"""
+        from protean.core.value_object import BaseValueObject
+
+        if not issubclass(value_object_cls, BaseValueObject):
+            raise IncorrectUsageError(
+                {
+                    "_value_object": [
+                        f"`{value_object_cls.__name__}` is not a valid Value Object"
+                    ]
+                }
+            )
+
     @property
     def value_object_cls(self):
         return self._value_object_cls
 
     def _resolve_to_cls(self, domain, value_object_cls, owner_cls):
-        assert isinstance(self.value_object_cls, str)
+        assert isinstance(self._value_object_cls, str)
+
+        # Validate the class being passed is a subclass of BaseValueObject
+        self._validate_value_object_cls(value_object_cls)
 
         self._value_object_cls = value_object_cls
 

tests/adapters/broker/redis_broker/tests.py

@@ -60,9 +60,7 @@ def test_event_message_structure(self, test_domain):
                 "metadata",
             ]
         )
-        assert (
-            json_message["type"] == "Redis Broker Tests.PersonAdded.v1"
-        )  #  FIXME Normalize Domain Name
+        assert json_message["type"] == "RedisBrokerTests.PersonAdded.v1"
         assert json_message["metadata"]["kind"] == "EVENT"
 
 

tests/command/test_command_field_types.py

@@ -0,0 +1,38 @@
+import pytest
+
+from protean import BaseAggregate, BaseCommand, BaseEntity
+from protean.exceptions import IncorrectUsageError
+from protean.fields import HasMany, HasOne, String
+
+
+class User(BaseAggregate):
+    email = String()
+    name = String()
+    account = HasOne("Account")
+    addresses = HasMany("Address")
+
+
+class Account(BaseEntity):
+    password_hash = String()
+
+
+class Address(BaseEntity):
+    street = String()
+    city = String()
+    state = String()
+    postal_code = String()
+
+
+def test_events_cannot_hold_associations():
+    with pytest.raises(IncorrectUsageError) as exc:
+
+        class Register(BaseCommand):
+            email = String()
+            name = String()
+            account = HasOne(Account)
+
+    assert exc.value.messages == {
+        "_event": [
+            "Commands cannot have associations. Remove account (HasOne) from class Register"
+        ]
+    }

tests/event/test_event_field_types.py

@@ -0,0 +1,38 @@
+import pytest
+
+from protean import BaseAggregate, BaseEntity, BaseEvent
+from protean.exceptions import IncorrectUsageError
+from protean.fields import HasMany, HasOne, String
+
+
+class User(BaseAggregate):
+    email = String()
+    name = String()
+    account = HasOne("Account")
+    addresses = HasMany("Address")
+
+
+class Account(BaseEntity):
+    password_hash = String()
+
+
+class Address(BaseEntity):
+    street = String()
+    city = String()
+    state = String()
+    postal_code = String()
+
+
+def test_events_cannot_hold_associations():
+    with pytest.raises(IncorrectUsageError) as exc:
+
+        class UserRegistered(BaseEvent):
+            email = String()
+            name = String()
+            account = HasOne(Account)
+
+    assert exc.value.messages == {
+        "_event": [
+            "Events cannot have associations. Remove account (HasOne) from class UserRegistered"
+        ]
+    }

tests/event/test_event_metadata.py

@@ -85,6 +85,41 @@ class UserLoggedIn(BaseEvent):
         event = UserLoggedIn(user_id=str(uuid4()))
         assert event._metadata.version == "v2"
 
+    def test_version_value_in_multiple_event_definitions(self, test_domain):
+        def version1():
+            class DummyEvent(BaseEvent):
+                user_id = Identifier(identifier=True)
+
+            return DummyEvent
+
+        def version2():
+            class DummyEvent(BaseEvent):
+                __version__ = "v2"
+                user_id = Identifier(identifier=True)
+
+            return DummyEvent
+
+        event_cls1 = version1()
+        event_cls2 = version2()
+
+        test_domain.register(event_cls1, part_of=User)
+        test_domain.register(event_cls2, part_of=User)
+        test_domain.init(traverse=False)
+
+        assert event_cls1.__version__ == "v1"
+        assert event_cls2.__version__ == "v2"
+
+        assert len(test_domain.registry.events) == 3  # Includes UserLoggedIn
+
+        assert (
+            test_domain.registry.events[fqn(event_cls1)].cls.__type__
+            == "Test.DummyEvent.v1"
+        )
+        assert (
+            test_domain.registry.events[fqn(event_cls2)].cls.__type__
+            == "Test.DummyEvent.v2"
+        )
+
 
 def test_event_metadata():
     user_id = str(uuid4())

tests/field/test_vo.py

@@ -0,0 +1,32 @@
+import pytest
+
+from protean import BaseAggregate, BaseEntity, BaseValueObject
+from protean.exceptions import IncorrectUsageError
+from protean.fields import String, ValueObject
+from protean.reflection import fields
+
+
+def test_value_object_associated_class(test_domain):
+    class Address(BaseValueObject):
+        street_address = String()
+
+    class User(BaseAggregate):
+        email = String()
+        address = ValueObject(Address)
+
+    assert fields(User)["address"].value_object_cls == Address
+
+
+def test_value_object_to_cls_is_always_a_base_value_object_subclass(test_domain):
+    class Address(BaseEntity):
+        street_address = String()
+
+    with pytest.raises(IncorrectUsageError) as exc:
+
+        class User(BaseAggregate):
+            email = String()
+            address = ValueObject(Address)
+
+    assert exc.value.messages == {
+        "_value_object": ["`Address` is not a valid Value Object"]
+    }