diff --git a/cog.toml b/cog.toml index 1be75dcb..cdd8b164 100644 --- a/cog.toml +++ b/cog.toml @@ -5,12 +5,12 @@ ignore_merge_commits = true # so we can do our own skip logic in workflow skip_ci = "[ci-skip]" pre_bump_hooks = [ - "uv version --no-sync {{version}}", - "git add :pyproject.toml :uv.lock" + "uv version --no-sync {{version}}", + "git add :pyproject.toml :uv.lock" ] post_bump_hooks = [ - "git push", - "git push origin {{version_tag}}", + "git push", + "git push origin {{version_tag}}", ] [changelog] @@ -21,8 +21,8 @@ owner = "gazorby" repository = "strawchemy" authors = [ - { signature = "gazorby", username = "gazorby" }, - { signature = "Matthieu", username = "gazorby" } + { signature = "gazorby", username = "gazorby" }, + { signature = "Matthieu", username = "gazorby" } ] [commit_types] @@ -42,10 +42,8 @@ test = { omit_from_changelog = true } [bump_profiles.manual] pre_bump_hooks = [ - "uv version --no-sync {{version}}", - "mise run lint", + "uv version --no-sync {{version}}", + "mise run lint", ] -post_bump_hooks = [ - "echo 'Manual bump from {{latest}} to {{version}}.'" -] +post_bump_hooks = ["echo 'Manual bump from {{latest}} to {{version}}.'"] diff --git a/src/strawchemy/mapper.py b/src/strawchemy/mapper.py index acc6bb70..ea791874 100644 --- a/src/strawchemy/mapper.py +++ b/src/strawchemy/mapper.py @@ -34,6 +34,7 @@ StrawchemyUpdateMutationField, StrawchemyUpsertMutationField, ) +from strawchemy.schema.mutation.input import EventRegistry from strawchemy.utils.registry import StrawberryRegistry if TYPE_CHECKING: @@ -108,6 +109,7 @@ def __init__( """ self.config = StrawchemyConfig(cast("SupportedDialect", config)) if isinstance(config, str) else config self.registry = StrawberryRegistry(strawberry_config or StrawberryConfig()) + self._event_registry = EventRegistry() strawberry_backend = StrawberrryDTOBackend( MappedStrawberryGraphQLDTO, auto_is_type_of=self.config.auto_is_type_of @@ -137,6 +139,7 @@ def __init__( order_by_factory=self.order_by_factory, filter_factory=self.filter_factory, distinct_on_factory=self.distinct_on_enum_factory, + event_registry=self._event_registry, ) # Decorators diff --git a/src/strawchemy/schema/mutation/field_builder.py b/src/strawchemy/schema/mutation/field_builder.py index 8734cd6a..699287d4 100644 --- a/src/strawchemy/schema/mutation/field_builder.py +++ b/src/strawchemy/schema/mutation/field_builder.py @@ -8,6 +8,13 @@ from strawberry.annotation import StrawberryAnnotation +from strawchemy.schema.mutation.fields import ( + StrawchemyCreateMutationField, + StrawchemyDeleteMutationField, + StrawchemyUpdateMutationField, + StrawchemyUpsertMutationField, +) + if TYPE_CHECKING: from collections.abc import Callable, Mapping, Sequence @@ -17,12 +24,7 @@ from strawchemy.config.base import StrawchemyConfig from strawchemy.schema.factories import DistinctOnEnumFactory from strawchemy.schema.factories.inputs import BooleanFilterFactory, OrderByFactory - from strawchemy.schema.mutation.fields import ( - StrawchemyCreateMutationField, - StrawchemyDeleteMutationField, - StrawchemyUpdateMutationField, - StrawchemyUpsertMutationField, - ) + from strawchemy.schema.mutation.input import EventRegistry from strawchemy.typing import AnyRepositoryType @@ -40,6 +42,8 @@ class MutationFieldBuilder: order_by_factory: OrderByFactory filter_factory: BooleanFilterFactory distinct_on_factory: DistinctOnEnumFactory + event_registry: EventRegistry + """Shared event registry owned by the Strawchemy instance, reused across mutation requests.""" def build( self, @@ -92,6 +96,13 @@ def build( namespace = self.registry_namespace_getter() type_annotation = StrawberryAnnotation.from_annotation(graphql_type, namespace) if graphql_type else None + # Inject the shared registry only for input mutation fields (create/update/upsert), + # not for the delete field which shares the `input_type` kwarg name for filter types. + if issubclass( + field_class, (StrawchemyCreateMutationField, StrawchemyUpdateMutationField, StrawchemyUpsertMutationField) + ): + field_specific_kwargs.setdefault("event_registry", self.event_registry) + field = field_class( config=self.config, repository_type=repository_type, diff --git a/src/strawchemy/schema/mutation/fields.py b/src/strawchemy/schema/mutation/fields.py index 72f9cc23..7448224b 100644 --- a/src/strawchemy/schema/mutation/fields.py +++ b/src/strawchemy/schema/mutation/fields.py @@ -21,6 +21,7 @@ from strawchemy.dto.strawberry import BooleanFilterDTO, EnumDTO from strawchemy.repository.strawberry.base import GraphQLResult + from strawchemy.schema.mutation.input import EventRegistry from strawchemy.typing import AnyMappedDTO, CreateOrUpdateResolverResult, ListResolverResult, MappedGraphQLDTO from strawchemy.validation import ValidationProtocol @@ -41,12 +42,14 @@ def __init__( input_type: type[MappedGraphQLDTO[T]], *args: Any, validation: ValidationProtocol[T] | None = None, + event_registry: EventRegistry | None = None, **kwargs: Any, ) -> None: super().__init__(*args, **kwargs) self.is_root_field = True self._input_type = input_type self._validation = validation + self._event_registry = event_registry class _StrawchemyMutationField: @@ -68,7 +71,7 @@ def _create_resolver( ) -> CreateOrUpdateResolverResult | Coroutine[CreateOrUpdateResolverResult, Any, Any]: repository = self._get_repository(info) try: - input_data = Input(data, self._validation) + input_data = Input(data, self._validation, registry=self._event_registry) except InputValidationError as error: return error.graphql_type() if self._is_repo_async(repository): @@ -111,7 +114,7 @@ def _upsert_resolver( ) -> CreateOrUpdateResolverResult | Coroutine[CreateOrUpdateResolverResult, Any, Any]: repository = self._get_repository(info) try: - input_data = Input(data, self._validation) + input_data = Input(data, self._validation, registry=self._event_registry) except InputValidationError as error: return error.graphql_type() if self._is_repo_async(repository): @@ -165,7 +168,7 @@ def _update_by_ids_resolver( ) -> CreateOrUpdateResolverResult | Coroutine[CreateOrUpdateResolverResult, Any, Any]: repository = self._get_repository(info) try: - input_data = Input(data, self._validation) + input_data = Input(data, self._validation, registry=self._event_registry) except InputValidationError as error: error_result = error.graphql_type() return [error_result] if isinstance(data, Sequence) else error_result @@ -179,7 +182,7 @@ def _update_by_filter_resolver( ) -> CreateOrUpdateResolverResult | Coroutine[CreateOrUpdateResolverResult, Any, Any]: repository = self._get_repository(info) try: - input_data = Input(data, self._validation) + input_data = Input(data, self._validation, registry=self._event_registry) except InputValidationError as error: return [error.graphql_type()] if self._is_repo_async(repository): diff --git a/src/strawchemy/schema/mutation/input.py b/src/strawchemy/schema/mutation/input.py index fdca28f6..5899f863 100644 --- a/src/strawchemy/schema/mutation/input.py +++ b/src/strawchemy/schema/mutation/input.py @@ -4,6 +4,7 @@ from collections.abc import Hashable, Iterator, Sequence from dataclasses import dataclass, field from typing import TYPE_CHECKING, Any, Generic, Literal, TypeAlias, TypeVar, cast, final +from weakref import WeakValueDictionary from sqlalchemy import event, inspect from sqlalchemy.orm import MapperProperty, RelationshipDirection, object_mapper @@ -30,8 +31,7 @@ from strawchemy.typing import MappedGraphQLDTO from strawchemy.validation.base import ValidationProtocol - -__all__ = ("Input", "InputModel", "LevelInput", "RelationType") +__all__ = ("EventRegistry", "Input", "InputModel", "LevelInput", "RelationInput", "RelationType") T = TypeVar("T", bound=MappedDTO[Any]) DeclarativeBaseT = TypeVar("DeclarativeBaseT", bound="DeclarativeBase") @@ -103,22 +103,19 @@ def __bool__(self) -> bool: @dataclass class RelationInput(_UnboundRelationInput): parent: DeclarativeBase = field(kw_only=True) + event_registry: EventRegistry = field(kw_only=True) def __post_init__(self) -> None: super().__post_init__() - - if self.relation_type is RelationType.TO_ONE: - event.listens_for(self.attribute, "set")(self._set_event) - else: - event.listens_for(self.attribute, "append")(self._append_event) - event.listens_for(self.attribute, "remove")(self._remove_event) + self.event_registry.register(self) @classmethod - def from_unbound(cls, unbound: _UnboundRelationInput, model: DeclarativeBase) -> Self: + def from_unbound(cls, unbound: _UnboundRelationInput, model: DeclarativeBase, registry: EventRegistry) -> Self: return cls( attribute=unbound.attribute, related=unbound.related, parent=model, + event_registry=registry, set_=unbound.set, add=unbound.add, remove=unbound.remove, @@ -129,7 +126,7 @@ def from_unbound(cls, unbound: _UnboundRelationInput, model: DeclarativeBase) -> upsert=unbound.upsert, ) - def _set_event(self, target: DeclarativeBase, value: DeclarativeBase | None, *_: Any, **__: Any) -> None: + def handle_set(self, value: DeclarativeBase | None) -> None: if value is None: return if _has_record(value): @@ -137,19 +134,77 @@ def _set_event(self, target: DeclarativeBase, value: DeclarativeBase | None, *_: else: self.create = [value] - def _append_event(self, target: DeclarativeBase, value: DeclarativeBase, *_: Any, **__: Any) -> None: + def handle_append(self, value: DeclarativeBase) -> None: if _has_record(value): self.add.append(value) else: self.create.append(value) - def _remove_event(self, target: DeclarativeBase, value: DeclarativeBase, *_: Any, **__: Any) -> None: + def handle_remove(self, value: DeclarativeBase) -> None: if _has_record(value): self.add = [model for model in self.add if model is not value] else: self.create = [model for model in self.create if model is not value] +def _new_entries() -> WeakValueDictionary[tuple[int, str], RelationInput]: + return WeakValueDictionary() + + +@dataclass +class EventRegistry: + """Routes SQLAlchemy relationship events to the RelationInput owning (target, attribute). + + A single set/append/remove listener is registered per relationship attribute for the + process lifetime. Each event is dispatched to the one RelationInput whose ``parent`` is + the event's ``target``, looked up in a weakly-held per-parent mapping. + """ + + _entries: WeakValueDictionary[tuple[int, str], RelationInput] = field(init=False, default_factory=_new_entries) + """Maps ``(id(parent), attr_key)`` to the owning RelationInput, held weakly. + + The value (``RelationInput``) strongly references ``parent``, so ``id(parent)`` + cannot be recycled while this entry is live — the key is therefore stable for the + entry's lifetime. + """ + _registry: set[MapperProperty[Any]] = field(init=False, default_factory=set) + """Relationship attributes whose dispatcher has already been registered.""" + + def register(self, relation: RelationInput) -> None: + """Wire the relation's attribute once and index the relation by its parent.""" + self._register(relation.attribute, relation.relation_type) + # Weak-ownership invariant: the registry holds entries weakly (by value), so a + # RelationInput that is no longer retained by its owning Input stops routing once + # collected. The owning Input strongly holds every relation it consumes via + # self.relations, so live relations are always reachable. + self._entries[(id(relation.parent), relation.attribute.key)] = relation + + def _register(self, attribute: MapperProperty[Any], relation_type: RelationType) -> None: + if attribute in self._registry: + return + if relation_type is RelationType.TO_ONE: + event.listens_for(attribute, "set")(self._dispatch_set) + else: + event.listens_for(attribute, "append")(self._dispatch_append) + event.listens_for(attribute, "remove")(self._dispatch_remove) + self._registry.add(attribute) + + def _get_input(self, target: DeclarativeBase, initiator: Any) -> RelationInput | None: + return self._entries.get((id(target), initiator.key)) + + def _dispatch_set(self, target: DeclarativeBase, value: DeclarativeBase, _oldvalue: Any, initiator: Any) -> None: + if (relation := self._get_input(target, initiator)) is not None: + relation.handle_set(value) + + def _dispatch_append(self, target: DeclarativeBase, value: DeclarativeBase, initiator: Any) -> None: + if (relation := self._get_input(target, initiator)) is not None: + relation.handle_append(value) + + def _dispatch_remove(self, target: DeclarativeBase, value: DeclarativeBase, initiator: Any) -> None: + if (relation := self._get_input(target, initiator)) is not None: + relation.handle_remove(value) + + @dataclass class _InputVisitor(VisitorProtocol[DeclarativeBaseT], Generic[DeclarativeBaseT, InputModel]): input_data: Input[InputModel] @@ -228,7 +283,7 @@ def model( delattr(model, attribute) for relation in self.current_relations: - self.input_data.add_relation(RelationInput.from_unbound(relation, model)) + self.input_data.add_relation(RelationInput.from_unbound(relation, model, self.input_data.registry)) self.current_relations.clear() # Return dict because .model_validate will be called at root level return model if level == 1 or self.input_data.validation is None else params @@ -250,8 +305,11 @@ def __init__( self, dtos: MappedGraphQLDTO[InputModel] | Sequence[MappedGraphQLDTO[InputModel]], _validation_: ValidationProtocol[InputModel] | None = None, + *, + registry: EventRegistry | None = None, **override: Any, ) -> None: + self.registry = registry if registry is not None else EventRegistry() self.max_level = 0 self.relations: list[RelationInput] = [] self.instances: list[InputModel] = [] @@ -305,6 +363,7 @@ def _add_non_input_relations( relation = RelationInput( attribute=relationship, parent=model, + event_registry=self.registry, level=_level, input_index=input_index, relation_type=relation_type, diff --git a/tests/unit/test_event_registry.py b/tests/unit/test_event_registry.py new file mode 100644 index 00000000..45a9d62b --- /dev/null +++ b/tests/unit/test_event_registry.py @@ -0,0 +1,150 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING, Any +from uuid import uuid4 + +from sqlalchemy import event, inspect + +from strawchemy.schema.mutation.input import EventRegistry, RelationInput +from strawchemy.schema.mutation.types import RelationType +from tests.unit.models import Color, Fruit + +if TYPE_CHECKING: + import pytest + from sqlalchemy.orm import RelationshipProperty + + +def _fruits_prop() -> RelationshipProperty[Any]: + """The Color.fruits relationship MapperProperty (to-many).""" + return inspect(Color).attrs["fruits"] + + +def _make_relation(registry: EventRegistry, parent: Color) -> RelationInput: + return RelationInput( + attribute=_fruits_prop(), + related=Fruit, + parent=parent, + relation_type=RelationType.TO_MANY, + event_registry=registry, + ) + + +def _color_prop() -> RelationshipProperty[Any]: + """The Fruit.color relationship MapperProperty (to-one).""" + return inspect(Fruit).attrs["color"] + + +def _make_to_one_relation(registry: EventRegistry, parent: Fruit) -> RelationInput: + return RelationInput( + attribute=_color_prop(), + related=Color, + parent=parent, + relation_type=RelationType.TO_ONE, + event_registry=registry, + ) + + +def test_append_event_updates_only_owning_relation() -> None: + """An append on one parent's collection fills only that parent's RelationInput create bucket.""" + registry = EventRegistry() + parent_a, parent_b = Color(name="A"), Color(name="B") + relation_a = _make_relation(registry, parent_a) + relation_b = _make_relation(registry, parent_b) + fruit_a = Fruit(name="A", color_id=uuid4(), sweetness=1, color=None) + fruit_b = Fruit(name="B", color_id=uuid4(), sweetness=1, color=None) + + parent_a.fruits.append(fruit_a) + parent_b.fruits.append(fruit_b) + + assert relation_a.create == [fruit_a] + assert relation_b.create == [fruit_b] + + +def test_remove_event_updates_only_owning_relation() -> None: + """A remove on one parent's collection clears only that parent's RelationInput create bucket.""" + registry = EventRegistry() + parent_a, parent_b = Color(name="A"), Color(name="B") + relation_a = _make_relation(registry, parent_a) + relation_b = _make_relation(registry, parent_b) + fruit_a = Fruit(name="A", color_id=uuid4(), sweetness=1, color=None) + fruit_b = Fruit(name="B", color_id=uuid4(), sweetness=1, color=None) + parent_a.fruits.append(fruit_a) + parent_b.fruits.append(fruit_b) + assert relation_a.create == [fruit_a] + assert relation_b.create == [fruit_b] + + parent_b.fruits.remove(fruit_b) + + assert relation_b.create == [] + assert relation_a.create == [fruit_a] # owning-relation isolation preserved + + +def test_set_event_updates_only_owning_relation() -> None: + """A set on one parent's to-one attribute fills only that parent's RelationInput create bucket.""" + registry = EventRegistry() + fruit_a = Fruit(name="A", color_id=uuid4(), sweetness=1, color=None) + fruit_b = Fruit(name="B", color_id=uuid4(), sweetness=1, color=None) + relation_a = _make_to_one_relation(registry, fruit_a) + relation_b = _make_to_one_relation(registry, fruit_b) + color = Color(name="Blue") # transient + + fruit_b.color = color # set event -> handle_set -> create == [color] + + assert relation_b.create == [color] + assert relation_a.create == [] + + +def test_attribute_listener_registered_once(monkeypatch: pytest.MonkeyPatch) -> None: + """Registering many relations for one attribute wires its listener a single time.""" + import strawchemy.schema.mutation.input as input_module + + real_listens_for = event.listens_for + append_registrations = 0 + + def counting_listens_for(target: Any, identifier: str, *args: Any, **kwargs: Any) -> Any: + nonlocal append_registrations + if target is _fruits_prop() and identifier == "append": + append_registrations += 1 + return real_listens_for(target, identifier, *args, **kwargs) + + monkeypatch.setattr(input_module.event, "listens_for", counting_listens_for) + + registry = EventRegistry() + _make_relation(registry, Color(name="A")) + _make_relation(registry, Color(name="B")) + _make_relation(registry, Color(name="C")) + + assert append_registrations == 1 + + +def test_strawchemy_reuses_one_registry_across_inputs(monkeypatch: pytest.MonkeyPatch) -> None: + """Two mutation inputs built from one Strawchemy share its registry and wire each attribute once.""" + import strawchemy.schema.mutation.input as input_module + from strawchemy import Strawchemy + from strawchemy.schema.mutation import Input + + strawchemy = Strawchemy("postgresql") + + @strawchemy.create_input(Color, include="all") + class ColorInput: ... + + registry = strawchemy._event_registry # noqa: SLF001 + + real_listens_for = event.listens_for + append_registrations = 0 + + def counting_listens_for(target: Any, identifier: str, *args: Any, **kwargs: Any) -> Any: + nonlocal append_registrations + if target is _fruits_prop() and identifier == "append": + append_registrations += 1 + return real_listens_for(target, identifier, *args, **kwargs) + + monkeypatch.setattr(input_module.event, "listens_for", counting_listens_for) + + for _ in range(2): + color_input = Input(ColorInput(name="Blue"), registry=registry) + color_input.instances[0].fruits.append(Fruit(name="Apple", color_id=uuid4(), sweetness=1, color=None)) + color_input.add_non_input_relations() + + # One wiring total despite two separate Input builds sharing the registry. + assert append_registrations == 1