diff --git a/docs/events.md b/docs/events.md index d3414d98..c1e352a3 100644 --- a/docs/events.md +++ b/docs/events.md @@ -71,6 +71,26 @@ human-readable display name, use the `Event` class explicitly: ``` +The same declaration works inside a {ref}`compound state ` body: + +```py +>>> class Journey(StateChart): +... class shire(State.Compound): +... bag_end = State(initial=True) +... green_dragon = State() +... +... visit_pub = Event(bag_end.to(green_dragon), name="Visit the pub") +... +... road = State(final=True) +... depart = Event(shire.to(road)) + +>>> sm = Journey() +>>> sm.send("visit_pub") +>>> set(sm.configuration_values) == {"shire", "green_dragon"} +True + +``` + (event-identity)= diff --git a/docs/releases/3.2.2.md b/docs/releases/3.2.2.md new file mode 100644 index 00000000..a361b2e2 --- /dev/null +++ b/docs/releases/3.2.2.md @@ -0,0 +1,80 @@ +# StateChart 3.2.2 + +*Not released yet* + +## Bug fixes in 3.2.2 + +### Event declarations inside `State.Compound` + +A `State.Compound` (or `State.Parallel`) class body only understood the assignment form of an +event declaration (`visit_pub = bag_end.to(green_dragon)`). The two other documented forms were +silently dropped: the name was bound to a detached object and the transition it wrapped stayed +{ref}`eventless `, firing as soon as its source state became active. + +The `Event` class now works inside a nested state body: + +```py +>>> from statemachine import Event, State, StateChart + +>>> class Journey(StateChart): +... class shire(State.Compound): +... bag_end = State(initial=True) +... green_dragon = State() +... +... visit_pub = Event(bag_end.to(green_dragon), name="Visit the pub") +... +... road = State(final=True) +... depart = Event(shire.to(road)) + +>>> sm = Journey() +>>> set(sm.configuration_values) == {"shire", "bag_end"} +True + +>>> sm.send("visit_pub") +>>> set(sm.configuration_values) == {"shire", "green_dragon"} +True + +>>> Journey.visit_pub.name +'Visit the pub' + +``` + +Before this fix, `Journey` started already in `green_dragon` and `visit_pub` was not among its +events. The attribute name now becomes the event `id`, an explicit `id` takes precedence (with +the attribute name still resolving to the event), `name` is kept as the display name, and the +`error_` / `done_state_` / `done_invoke_` prefixes expand to their dotted form. + +The same applies to the `@.to()` decorator, which declares an event and its +inline action at once. Inside a compound body it registered no event and never ran its body: + +```py +>>> class Gate(StateChart): +... class gate(State.Compound): +... locked = State(initial=True) +... unlocked = State() +... +... push = unlocked.to(locked) +... +... @locked.to(unlocked) +... def coin(self): +... return "accepted" +... +... broken = State(final=True) +... smash = gate.to(broken) + +>>> sm = Gate() +>>> sm.send("coin") +'accepted' + +>>> set(sm.configuration_values) == {"gate", "unlocked"} +True + +``` + +An `Event` declared inside a nested body with no transitions at all (`knock = Event()`) still +differs from the top-level form: it is reachable as a class attribute but is not added to the +machine's event list. + +Reported by [@Dolecor](https://github.com/Dolecor). + +[#643](https://github.com/fgmacedo/python-statemachine/issues/643). diff --git a/docs/releases/index.md b/docs/releases/index.md index 28929df3..f62455f7 100644 --- a/docs/releases/index.md +++ b/docs/releases/index.md @@ -16,6 +16,7 @@ Requires Python 3.10+. ```{toctree} :maxdepth: 2 +3.2.2 3.2.1 3.2.0 3.1.2 diff --git a/statemachine/class_body.py b/statemachine/class_body.py new file mode 100644 index 00000000..0d8d185d --- /dev/null +++ b/statemachine/class_body.py @@ -0,0 +1,59 @@ +from typing import TYPE_CHECKING +from typing import Any + +from .event import Event +from .state import HistoryState +from .state import State +from .states import States +from .transition import Transition +from .transition_list import TransitionList + +if TYPE_CHECKING: + from typing import Protocol + + class ClassBodyReader(Protocol): + """One method per declaration form that :func:`read` recognizes. + + Adding a form here is what forces every reader to answer for it. + """ + + def on_states(self, states: States) -> None: ... + + def on_history(self, key: str, state: HistoryState) -> None: ... + + def on_state(self, key: str, state: State) -> None: ... + + def on_transitions(self, key: str, transitions: "Transition | TransitionList") -> None: ... + + def on_event(self, key: str, declared: Event) -> None: ... + + def on_decorated(self, key: str, func: Any) -> None: ... + + def on_other(self, key: str, value: Any) -> None: ... + + +def read(attrs: "dict[str, Any]", reader: "ClassBodyReader") -> None: + """Dispatch each entry of a statechart class body to ``reader``. + + A statechart is declared in two kinds of class body: a :ref:`StateChart` subclass and a + nested ``State.Compound`` / ``State.Parallel``. Both accept the same forms, so recognition + lives here once and each reader supplies only what it does with a form. + + Order is significant: a ``HistoryState`` is a ``State``, and an ``Event`` is a callable + ``str``, so both would be captured by a later branch. + """ + for key, value in attrs.items(): + if isinstance(value, States): + reader.on_states(value) + elif isinstance(value, HistoryState): + reader.on_history(key, value) + elif isinstance(value, State): + reader.on_state(key, value) + elif isinstance(value, (Transition, TransitionList)): + reader.on_transitions(key, value) + elif isinstance(value, Event): + reader.on_event(key, value) + elif getattr(value, "attr_name", None): + reader.on_decorated(key, value) + else: + reader.on_other(key, value) diff --git a/statemachine/factory.py b/statemachine/factory.py index 3bced620..811f10ce 100644 --- a/statemachine/factory.py +++ b/statemachine/factory.py @@ -5,6 +5,7 @@ from .callbacks import CallbackGroup from .callbacks import CallbackPriority from .callbacks import CallbackSpecList +from .class_body import read from .event import Event from .event import _expand_event_id from .exceptions import InvalidDefinition @@ -15,8 +16,6 @@ from .i18n import _ from .state import State from .states import States -from .transition import Transition -from .transition_list import TransitionList class StateMachineMetaclass(type): @@ -298,43 +297,8 @@ def add_inherited(cls, bases): for event in events: cls.add_event(event=Event(id=event.id, name=event.name)) - def add_from_attributes(cls, attrs): # noqa: C901 - for key, value in attrs.items(): - if isinstance(value, States): - cls._add_states_from_dict(value) - if isinstance(value, State): - cls.add_state(key, value) - elif isinstance(value, (Transition, TransitionList)): - event_id = _expand_event_id(key) - cls.add_event(event=Event(transitions=value, id=event_id)) - elif isinstance(value, (Event,)): - if value._has_real_id: - event_id = value.id - else: - event_id = _expand_event_id(key) - new_event = Event( - transitions=value._transitions, - id=event_id, - name=value.name, - ) - cls.add_event(event=new_event, old_event=value) - # Ensure the event is accessible by the Python attribute name - if event_id != key: - setattr(cls, key, new_event) - elif getattr(value, "attr_name", None): - cls._add_unbounded_callback(key, value) - - def _add_states_from_dict(cls, states): - for state_id, state in states.items(): - cls.add_state(state_id, state) - - def _add_unbounded_callback(cls, attr_name, func): - # if func is an event, the `attr_name` will be replaced by an event trigger, - # so we'll also give the ``func`` a new unique name to be used by the callback - # machinery that is stored at ``func.attr_name`` - setattr(cls, func.attr_name, func) - if func.is_event: - cls.add_event(event=Event(func._transitions, id=attr_name)) + def add_from_attributes(cls, attrs): + read(attrs, _StateChartBody(cls)) def add_state(cls, id, state: State): state._set_id(id) @@ -390,3 +354,40 @@ def _update_event_references(cls): @property def events(self): return list(self._events) + + +class _StateChartBody: + """Registers a statechart class body on the class under construction.""" + + def __init__(self, cls: "StateMachineMetaclass") -> None: + self.cls = cls + + def on_states(self, states: States) -> None: + for state_id, state in states.items(): + self.cls.add_state(state_id, state) + + def on_state(self, key: str, state: State) -> None: + self.cls.add_state(key, state) + + on_history = on_state + """A top-level history state is registered as an ordinary state.""" + + def on_transitions(self, key: str, transitions: Any) -> None: + self.cls.add_event(event=Event(transitions=transitions, id=_expand_event_id(key))) + + def on_event(self, key: str, declared: Event) -> None: + event_id = declared.id if declared._has_real_id else _expand_event_id(key) + new_event = Event(transitions=declared._transitions, id=event_id, name=declared.name) + self.cls.add_event(event=new_event, old_event=declared) + if event_id != key: + setattr(self.cls, key, new_event) + + def on_decorated(self, key: str, func: Any) -> None: + # The attribute name is taken over by the event trigger, so the callback machinery + # reaches the function through the unique name it stored at ``func.attr_name``. + setattr(self.cls, func.attr_name, func) + if func.is_event: + self.cls.add_event(event=Event(func._transitions, id=key)) + + def on_other(self, key: str, value: Any) -> None: + """Anything else is already a plain class attribute.""" diff --git a/statemachine/state.py b/statemachine/state.py index 065cc52a..422c52ae 100644 --- a/statemachine/state.py +++ b/statemachine/state.py @@ -8,6 +8,7 @@ from .callbacks import CallbackGroup from .callbacks import CallbackPriority from .callbacks import CallbackSpecList +from .event import Event from .event import _expand_event_id from .exceptions import InvalidDefinition from .i18n import _ @@ -55,6 +56,52 @@ def __call__(self, *states: "State | NestedStateFactory", **kwargs): return transitions +class _NestedBody: + """Collects a nested state class body into the arguments of the :ref:`State` it becomes. + + Unlike :class:`statemachine.factory._StateChartBody`, this reader has no class to register + on. Everything that is not a substate is handed back as a callback for + ``StateMachineMetaclass._unpack_builders_callbacks`` to place on the owning statechart. + """ + + def __init__(self) -> None: + self.states: list[State] = [] + self.history: list[HistoryState] = [] + self.callbacks: dict = {} + + def on_states(self, states: Any) -> None: + for state_id, state in states.items(): + state._set_id(state_id) + self.states.append(state) + + def on_history(self, key: str, state: "HistoryState") -> None: + state._set_id(key) + self.history.append(state) + + def on_state(self, key: str, state: "State") -> None: + state._set_id(key) + self.states.append(state) + + def on_transitions(self, key: str, transitions: "Transition | TransitionList") -> None: + transitions.add_event(_expand_event_id(key)) + + def on_event(self, key: str, declared: "Event") -> None: + event_id = declared.id if declared._has_real_id else _expand_event_id(key) + event = Event(id=event_id, name=declared.name) + if declared._transitions is not None: + declared._transitions.add_event(event) + self.callbacks[key] = event + + def on_decorated(self, key: str, func: Any) -> None: + if func.is_event: + func._transitions.add_event(key) + self.callbacks[func.attr_name] = func + + def on_other(self, key: str, value: Any) -> None: + if callable(value): + self.callbacks[key] = value + + class NestedStateFactory(type): def __new__( # type: ignore [misc] cls, classname, bases, attrs, name="", **kwargs @@ -70,30 +117,18 @@ def __new__( # type: ignore [misc] inherited_kwargs.update(getattr(base, "_factory_kwargs", {})) inherited_kwargs.update(kwargs) - # Lazy import to avoid circular dependency (states.py imports state.py) - from .states import States - - states = [] - history = [] - callbacks = {} - for key, value in attrs.items(): - if isinstance(value, States): - for state_id, state in value.items(): - state._set_id(state_id) - states.append(state) - elif isinstance(value, HistoryState): - value._set_id(key) - history.append(value) - elif isinstance(value, State): - value._set_id(key) - states.append(value) - elif isinstance(value, TransitionList): - value.add_event(_expand_event_id(key)) - elif callable(value): - callbacks[key] = value + # Lazy import to avoid circular dependency (class_body.py imports state.py) + from .class_body import read + + body = _NestedBody() + read(attrs, body) return State( - name=name, states=states, history=history, _callbacks=callbacks, **inherited_kwargs + name=name, + states=body.states, + history=body.history, + _callbacks=body.callbacks, + **inherited_kwargs, ) @classmethod diff --git a/tests/test_state.py b/tests/test_state.py index 2e2d7f1c..e87cfc04 100644 --- a/tests/test_state.py +++ b/tests/test_state.py @@ -1,5 +1,6 @@ import pytest from statemachine.orderedset import OrderedSet +from statemachine.states import States from statemachine import State from statemachine import StateChart @@ -83,3 +84,11 @@ def test_ordered_set_union(): s1 = OrderedSet([1, 2]) result = s1.union([3, 4], [5, 6]) assert list(result) == [1, 2, 3, 4, 5, 6] + + +def test_states_getattr_unknown_name(): + """States exposes its members as attributes and rejects anything else.""" + states = States({"draft": State("Draft")}) + assert states.draft.name == "Draft" + with pytest.raises(AttributeError, match="published not found in States"): + _ = states.published diff --git a/tests/test_statechart_compound.py b/tests/test_statechart_compound.py index 1506e1f5..2c362fbf 100644 --- a/tests/test_statechart_compound.py +++ b/tests/test_statechart_compound.py @@ -13,6 +13,7 @@ import pytest from statemachine.states import States +from statemachine import Event from statemachine import State from statemachine import StateChart from tests.machines.compound.middle_earth_journey import MiddleEarthJourney @@ -235,7 +236,10 @@ class wrapper(State.Compound): await sm_runner.processing_loop(sm) assert {"done"} == set(sm.configuration_values) - async def test_error_execution_inside_compound(self, sm_runner): + @pytest.mark.parametrize( + "declare", [lambda transitions: transitions, Event], ids=["bare", "Event"] + ) + async def test_error_execution_inside_compound(self, sm_runner, declare): """error_execution inside a compound body registers error.execution event.""" def raise_error(): @@ -246,15 +250,19 @@ class active(State.Compound): ok = State(initial=True) failing = State() - trigger = ok.to(failing, on=raise_error) + trigger = declare(ok.to(failing, on=raise_error)) errored = State() - error_execution = failing.to(errored) + error_execution = declare(failing.to(errored)) done = State(final=True) finish = active.to(done) + assert "error.execution" in [event.id for event in ErrorInCompound.events] + sm = await sm_runner.start(ErrorInCompound) + assert "ok" in sm.configuration_values + await sm_runner.send(sm, "trigger") assert "errored" in sm.configuration_values @@ -302,3 +310,168 @@ class inner(State.Compound): await sm_runner.send(sm, "inner_to_baz_bar") assert {OuterStates.BAR} == set(sm.configuration_values) + + +@pytest.mark.timeout(5) +class TestEventClassInsideCompound: + """The ``Event`` class inside a ``State.Compound`` body (#643).""" + + async def test_event_class_declares_a_named_event(self, sm_runner): + """``Event()`` binds the event instead of leaving it eventless.""" + + class QuirkyJourney(StateChart): + class shire(State.Compound): + bag_end = State(initial=True) + green_dragon = State() + + visit_pub = Event(bag_end.to(green_dragon)) + + road = State(final=True) + depart = Event(shire.to(road)) + + assert [event.id for event in QuirkyJourney.events] == ["visit_pub", "depart"] + + sm = await sm_runner.start(QuirkyJourney) + assert {"shire", "bag_end"} == set(sm.configuration_values) + + await sm_runner.send(sm, "visit_pub") + assert {"shire", "green_dragon"} == set(sm.configuration_values) + + def test_display_name_is_preserved(self): + class NamedEvent(StateChart): + class shire(State.Compound): + bag_end = State(initial=True) + green_dragon = State(final=True) + + visit_pub = Event(bag_end.to(green_dragon), name="Visit the pub") + + assert NamedEvent.visit_pub.id == "visit_pub" + assert NamedEvent.visit_pub.name == "Visit the pub" + + async def test_explicit_id_is_reachable_by_both_names(self, sm_runner): + """An explicit ``id`` wins over the attribute name, which still resolves.""" + + class ExplicitId(StateChart): + class shire(State.Compound): + bag_end = State(initial=True) + green_dragon = State(final=True) + + visit_pub = Event(bag_end.to(green_dragon), id="pub.visit") + + assert [event.id for event in ExplicitId.events] == ["pub.visit"] + assert ExplicitId.visit_pub.id == "pub.visit" + + sm = await sm_runner.start(ExplicitId) + await sm_runner.send(sm, "pub.visit") + assert {"shire", "green_dragon"} == set(sm.configuration_values) + + async def test_combined_transitions(self, sm_runner): + class Wandering(StateChart): + class shire(State.Compound): + bag_end = State(initial=True) + green_dragon = State() + + wander = Event(bag_end.to(green_dragon) | green_dragon.to(bag_end)) + + road = State(final=True) + depart = Event(shire.to(road)) + + sm = await sm_runner.start(Wandering) + await sm_runner.send(sm, "wander") + assert "green_dragon" in sm.configuration_values + + await sm_runner.send(sm, "wander") + assert "bag_end" in sm.configuration_values + + async def test_event_inside_parallel_region(self, sm_runner): + class WarOfTheRing(StateChart): + class war(State.Parallel): + class quest(State.Compound): + start = State(initial=True) + end = State(final=True) + + go = Event(start.to(end)) + + class battle(State.Compound): + fighting = State(initial=True) + won = State(final=True) + + victory = Event(fighting.to(won)) + + sm = await sm_runner.start(WarOfTheRing) + assert {"war", "quest", "start", "battle", "fighting"} == set(sm.configuration_values) + + await sm_runner.send(sm, "go") + await sm_runner.send(sm, "victory") + assert {"war", "quest", "end", "battle", "won"} == set(sm.configuration_values) + + def test_event_without_transitions_is_reachable_but_unregistered(self): + """A transition-less ``Event`` gets its id from the attribute name. + + Having no transitions, it never reaches the machine's event list -- unlike the + top-level form, which registers it. + """ + + class Placeholder(StateChart): + class shire(State.Compound): + bag_end = State(initial=True) + green_dragon = State(final=True) + + visit_pub = bag_end.to(green_dragon) + knock = Event(name="Knock on the door") + + assert Placeholder.knock.id == "knock" + assert Placeholder.knock.name == "Knock on the door" + assert "knock" not in [event.id for event in Placeholder.events] + + +@pytest.mark.timeout(5) +class TestDecoratorEventInsideCompound: + """The ``@.to()`` decorator inside a ``State.Compound`` body.""" + + async def test_decorator_declares_a_named_event(self, sm_runner): + """The decorated name becomes the event, and its body runs as the ``on`` action.""" + + class Gate(StateChart): + class gate(State.Compound): + locked = State(initial=True) + unlocked = State() + + push = unlocked.to(locked) + + @locked.to(unlocked) + def coin(self): + return "accepted" + + broken = State(final=True) + smash = gate.to(broken) + + assert "coin" in [event.id for event in Gate.events] + + sm = await sm_runner.start(Gate) + assert "locked" in sm.configuration_values + + assert await sm_runner.send(sm, "coin") == "accepted" + assert "unlocked" in sm.configuration_values + + async def test_decorated_callback_is_not_an_event(self, sm_runner): + """``@.on`` keeps declaring a plain callback, not a new event.""" + + log = [] + + class Gate(StateChart): + class gate(State.Compound): + locked = State(initial=True) + unlocked = State(final=True) + + coin = locked.to(unlocked) + + @coin.on + def clink(self): + log.append("clink") + + assert [event.id for event in Gate.events] == ["coin"] + + sm = await sm_runner.start(Gate) + await sm_runner.send(sm, "coin") + assert log == ["clink"]