Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix to ensure associations can only link to other entities #418

Merged
merged 1 commit into from
May 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/core-concepts/building-blocks/aggregates.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# Aggregates

2 changes: 2 additions & 0 deletions docs/core-concepts/building-blocks/entities.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# Entities

Entities in one domain can be a Value Object in another. For example, a seat is
an Entity in the Theatre domain if the theatre allocate seat numbers to patrons.
If visitors are not allotted specific seats, then a seat can be considered a
Expand Down
10 changes: 10 additions & 0 deletions docs/guides/domain-definition/fields/association-fields.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
# Association Fields

Association fields in Protean are designed to represent and manage
relationships between different domain models. They facilitate the modeling of
complex relationships, while encapsulating the technical aspects of persisting
data.

Note that while Aggregates and Entities can manage associations, they can
never link to another Aggregate directly. Aggregates are transaction boundaries
and no transaction should span across aggregates. Read more in
[Aggregate concepts](../../../core-concepts/building-blocks/aggregates.md).

## `HasOne`

Represents an one-to-one association between an aggregate and its entities.
Expand Down
4 changes: 3 additions & 1 deletion docs/guides/domain-definition/identity.md
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
# Identity
# Identity

When IDENTITY_TYPE is INTEGER, Strings that have valid integers or UUIDs are allowed
34 changes: 34 additions & 0 deletions src/protean/domain/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -561,6 +561,40 @@ def _replace_element_by_class(self, new_element_cls):
aggregate_record.qualname
].cls = new_element_cls

def _validate_domain(self):
"""A method to validate the domain for correctness, called just before the domain is activated."""
# Check if all references are resolved
if self._pending_class_resolutions:
raise ConfigurationError(
{
"element": f"Unresolved references in domain {self.name}",
"unresolved": self._pending_class_resolutions,
}
)

# Check if `HasOne` and `HasMany` fields are linked to entities and not aggregates
for _, aggregate in self.registry.aggregates.items():
for _, field_obj in declared_fields(aggregate.cls).items():
if isinstance(field_obj, (HasOne, HasMany)):
if isinstance(field_obj.to_cls, str):
raise IncorrectUsageError(
{
"element": (
f"Unresolved target `{field_obj.to_cls}` for field "
f"`{aggregate.__name__}:{field_obj.name}`"
)
}
)
if field_obj.to_cls.element_type != DomainObjects.ENTITY:
raise IncorrectUsageError(
{
"element": (
f"Field `{field_obj.field_name}` in `{aggregate.cls.__name__}` "
"is not linked to an Entity class"
)
}
)

######################
# Element Decorators #
######################
Expand Down
3 changes: 3 additions & 0 deletions src/protean/domain/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,9 @@ def push(self):
# This call raises an exception if all references are not resolved
self.domain._resolve_references()

# Run Validations
self.domain._validate_domain()

def pop(self, exc=_sentinel):
"""Pops the domain context."""
try:
Expand Down
2 changes: 1 addition & 1 deletion src/protean/fields/association.py
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ class Association(FieldBase, FieldDescriptorMixin, FieldCacheMixin):
def __init__(self, to_cls, via=None, **kwargs):
super().__init__(**kwargs)

self.to_cls = to_cls # FIXME Test that `to_cls` contains a corresponding `Reference` field
self.to_cls = to_cls
self.via = via

# FIXME Find an elegant way to avoid these declarations in associations
Expand Down
5 changes: 2 additions & 3 deletions tests/aggregate/test_aggregate_abstraction.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import pytest

from protean import BaseAggregate
from protean.exceptions import NotSupportedError
from protean.fields import String
from protean.reflection import declared_fields

from .elements import AbstractRole, ConcreteRole
Expand All @@ -23,9 +25,6 @@ def test_that_concrete_entities_can_be_created_from_abstract_entities_through_in
assert concrete_role.foo == "Titan"

def test_that_abstract_entities_can_be_created_with_annotations(self, test_domain):
from protean import BaseAggregate
from protean.fields import String

class CustomBaseClass(BaseAggregate):
foo = String(max_length=25)

Expand Down
7 changes: 7 additions & 0 deletions tests/context/tests.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import mock
import pytest

from protean.globals import current_domain, g
Expand Down Expand Up @@ -152,3 +153,9 @@ def test_domain_context_globals_not_shared(self, test_domain):

with test_domain.domain_context(foo="baz"):
assert g.foo == "baz"

def test_domain_context_activation_calls_validate_domain(self, test_domain):
mock_validate_domain = mock.Mock()
test_domain._validate_domain = mock_validate_domain
with test_domain.domain_context():
mock_validate_domain.assert_called_once()
14 changes: 13 additions & 1 deletion tests/field/test_has_many.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import pytest

from protean import BaseAggregate, BaseEntity
from protean.exceptions import IncorrectUsageError
from protean.fields import HasMany, String
from protean.reflection import attributes, declared_fields

Expand All @@ -23,7 +24,7 @@ def register_elements(test_domain):
test_domain.register(Comment)


class TestHasManyFields:
class TestHasManyFieldInProperties:
def test_that_has_many_field_appears_in_fields(self):
assert "comments" in declared_fields(Post)

Expand All @@ -37,6 +38,17 @@ def test_that_reference_field_does_not_appear_in_attributes(self):
assert "post" not in attributes(Comment)


class TestHasManyField:
def test_that_has_many_field_cannot_be_linked_to_aggregates(self, test_domain):
class InvalidAggregate(BaseAggregate):
post = HasMany(Post)

test_domain.register(InvalidAggregate)
with pytest.raises(IncorrectUsageError):
# The `post` field is invalid because it is linked to another Aggregate
test_domain._validate_domain()


class TestHasManyPersistence:
def test_that_has_many_field_is_persisted_along_with_aggregate(self, test_domain):
comment = Comment(content="First Comment")
Expand Down
14 changes: 13 additions & 1 deletion tests/field/test_has_one.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import pytest

from protean import BaseAggregate, BaseEntity
from protean.exceptions import IncorrectUsageError
from protean.fields import HasOne, Reference, String
from protean.reflection import attributes, declared_fields

Expand All @@ -24,7 +25,7 @@ def register(test_domain):
test_domain.register(Author)


class TestHasOneFields:
class TestHasOneFieldsInProperties:
def test_that_has_one_field_appears_in_fields(self):
assert "author" in declared_fields(Book)

Expand All @@ -38,6 +39,17 @@ def test_that_reference_field_does_not_appear_in_attributes(self):
assert "book" not in attributes(Author)


class TestHasOneField:
def test_that_has_one_field_cannot_be_linked_to_aggregates(self, test_domain):
class InvalidAggregate(BaseAggregate):
author = HasOne("Book")

test_domain.register(InvalidAggregate)
with pytest.raises(IncorrectUsageError):
# The `author` HasOne field is invalid because it is linked to an Aggregate
test_domain._validate_domain()


class TestHasOnePersistence:
def test_that_has_one_field_is_persisted_along_with_aggregate(self, test_domain):
author = Author(name="John Doe")
Expand Down
Loading