fix: Fix parallel enter/exit and checks

Author: fgmacedo

docs/releases/3.0.0.md

@@ -4,11 +4,21 @@
 
 ## What's new in 3.0.0
 
-Statecharts are there! Now the library has support for Compound and Parallel states.
+Statecharts are there! 🎉
 
-### Python compatibility in 3.0.0
+Statecharts are a powerful extension to state machines, in a way to organize complex reactive systems as a hierarchical state machine. They extend the concept of state machines by adding two new kinds of states: **parallel states** and **compound states**.
 
-StateMachine 3.0.0 supports Python 3.9, 3.10, 3.11, 3.12, and 3.13.
+**Parallel states** are states that can be active at the same time. They are useful for separating the state machine in multiple orthogonal state machines that can be active at the same time.
+
+**Compound states** are states that have inner states. They are useful for breaking down complex state machines into multiple simpler ones.
+
+The support for statecharts in this release follows the [SCXML specification](https://www.w3.org/TR/scxml/)*, which is a W3C standard for statecharts notation. Adhering as much as possible to this specification ensures compatibility with other tools and platforms that also implement SCXML, but more important,
+sets a standard on the expected behaviour that the library should assume on various edge cases, enabling easier integration and interoperability in complex systems.
+
+To verify the standard adoption, now the automated tests suite includes several `.scxml` testcases provided by the W3C group. Many thanks for this amazing work! Some of the tests are still failing, and some of the tags are still not implemented like `<invoke>` , in such cases, we've added an `xfail` mark by including a `test<number>.scxml.md` markdown file with details of the execution output.
+
+While these are exiting news for the library and our community, it also introduces several backwards incompatible changes. Due to the major version release, the new behaviour is assumed by default, but we put
+a lot of effort to minimize the changes needed in your codebase, and also introduced a few configuration options that you can enable to restore the old behaviour when possible. The following sections navigate to the new features and includes a migration guide.
 
 
 ### Create state machine class from a dict definition
@@ -73,9 +83,9 @@ A not so usefull example:
 
 ### Event matching following SCXML spec
 
-Now events matching follows the SCXML spec.
+Now events matching follows the [SCXML spec](https://www.w3.org/TR/scxml/#events):
 
-For example, a transition with an `event` attribute of `"error foo"` will match event names `error`, `error.send`, `error.send.failed`, etc. (or `foo`, `foo.bar` etc.)
+> For example, a transition with an `event` attribute of `"error foo"` will match event names `error`, `error.send`, `error.send.failed`, etc. (or `foo`, `foo.bar` etc.)
 but would not match events named `errors.my.custom`, `errorhandler.mistake`, `error.send` or `foobar`.
 
 An event designator consisting solely of `*` can be used as a wildcard matching any sequence of tokens, and thus any event.
@@ -90,6 +100,7 @@ TODO: Example of delayed events
 Also, delayed events can be revoked by it's `send_id`.
 
 
+
 ## Bugfixes in 3.0.0
 
 - Fixes [#XXX](https://github.com/fgmacedo/python-statemachine/issues/XXX).
@@ -100,22 +111,27 @@ TODO.
 
 ## Backward incompatible changes in 3.0
 
-- Dropped support for Python `3.7` and `3.8`. If you need support for these versios use the 2.* series.
+
+### Python compatibility in 3.0.0
+
+We've dropped support for Python `3.7` and `3.8`. If you need support for these versios use the 2.* series.
+
+StateMachine 3.0.0 supports Python 3.9, 3.10, 3.11, 3.12, and 3.13.
 
 
-## Non-RTC model removed
+### Non-RTC model removed
 
 This option was deprecated on version 2.3.2. Now all new events are put on a queue before being processed.
 
 
-## Multiple current states
+### Multiple current states
 
 Due to the support of compound and parallel states, it's now possible to have multiple active states at the same time.
 
-This introduces an impedance mismatch into the old public API, specifically, `sm.current_state` is deprecated and `sm.current_state_value` can returns a flat value if no compound state or a `list` instead.
+This introduces an impedance mismatch into the old public API, specifically, `sm.current_state` is deprecated and `sm.current_state_value` can returns a flat value if no compound state or a `set` instead.
 
 ```{note}
-To allow a smooth migration, these properties still work as before if there's no compound states in the state machine definition.
+To allow a smooth migration, these properties still work as before if there's no compound/parallel states in the state machine definition.
 ```
 
 Old
@@ -130,9 +146,63 @@ New
     def current_state(self) -> "State | MutableSet[State]":
 ```
 
-We recomend using the new `sm.configuration` that has a stable API returning an `OrderedSet` on all cases:
+We **strongly** recomend using the new `sm.configuration` that has a stable API returning an `OrderedSet` on all cases:
 
 ```py
     @property
     def configuration(self) -> OrderedSet["State"]:
 ```
+
+### Entering and exiting states
+
+Previous versions performed an atomic update of the active state just after the execution of the transition `on` actions.
+
+Now, we follow the [SCXML spec](https://www.w3.org/TR/scxml/#SelectingTransitions):
+
+> To execute a microstep, the SCXML Processor MUST execute the transitions in the corresponding optimal enabled transition set. To execute a set of transitions, the SCXML Processor MUST first exit all the states in the transitions' exit set in exit order. It MUST then execute the executable content contained in the transitions in document order. It MUST then enter the states in the transitions' entry set in entry order.
+
+This introduces backward-incompatible changes, as previously, the `current_state` was never empty, allowing queries on `sm.current_state` or `sm.<any_state>.is_active` even while executing an `on` transition action.
+
+Now, by default, during a transition, all states in the exit set are exited first, performing the `before` and `exit` callbacks. The `on` callbacks are then executed in an intermediate state that contains only the states that will not be exited, which can be an empty set. Following this, the states in the enter set are entered, with `enter` callbacks executed for each state in document order, and finally, the `after` callbacks are executed with the state machine in the final new configuration.
+
+We have added two new keyword arguments available only in the `on` callbacks to assist with queries that were performed against `sm.current_state` or active states using `<state>.is_active`:
+
+- `previous_configuration: OrderedSet[State]`: Contains the set of states that were active before the microstep was taken.
+- `new_configuration: OrderedSet[State]`: Contains the set of states that will be active after the microstep finishes.
+
+Additionally, you can create a state machine instance by passing `atomic_configuration_update=True` (default `False`) to restore the old behavior. When set to `False`, the `sm.configuration` will be updated only once per microstep, just after the `on` callbacks with the `new_configuration`, the set of states that should be active after the microstep.
+
+
+Consider this example that needs to be upgraded:
+
+```py
+class ApprovalMachine(StateMachine):
+    "A workflow"
+
+    requested = State(initial=True)
+    accepted = State()
+    rejected = State()
+    completed = State(final=True)
+
+    validate = (
+        requested.to(accepted, cond="is_ok") | requested.to(rejected) | accepted.to(completed)
+    )
+    retry = rejected.to(requested)
+
+    def on_validate(self):
+        if self.accepted.is_active and self.model.is_ok():
+            return "congrats!"
+
+```
+The `validate` event is bound to several transitions, and the `on_validate` is expected to return `congrats` only when the state machine was with the `accepted` state active before the event occurs. In the old behavior, checking for `accepted.is_active` evaluates to `True` because the state were not exited before the `on` callback.
+
+Due to the new behaviour, at the time of the `on_validate` call, the state machine configuration (a.k.a the current set of active states) is empty. So at this point in time `accepted.is_active` evaluates to `False`. To mitigate this case, now you can request one of the two new keyword arguments:  `previous_configuration` and `new_configration` in `on` callbacks.
+
+New way using `previous_configuration`:
+
+```py
+def on_validate(self, previous_configuration):
+    if self.accepted in previous_configuration and self.model.is_ok():
+        return "congrats!"
+
+```

docs/transitions.md

@@ -84,7 +84,7 @@ Syntax:
 >>> draft = State("Draft")
 
 >>> draft.to.itself()
-TransitionList([Transition('Draft', 'Draft', event=[], internal=False)])
+TransitionList([Transition('Draft', 'Draft', event=[], internal=False, initial=False)])
 
 ```
 
@@ -101,7 +101,7 @@ Syntax:
 >>> draft = State("Draft")
 
 >>> draft.to.itself(internal=True)
-TransitionList([Transition('Draft', 'Draft', event=[], internal=True)])
+TransitionList([Transition('Draft', 'Draft', event=[], internal=True, initial=False)])
 
 ```
 

statemachine/contrib/diagram.py

@@ -176,6 +176,7 @@ def get_graph(self):
         return graph
 
     def _graph_states(self, state, graph, is_root=False):
+        # TODO: handle parallel states in diagram
         initial_node = self._initial_node(state)
         initial_subgraph = pydot.Subgraph(
             graph_name=f"{initial_node.get_name()}_initial",

statemachine/engines/base.py

@@ -79,7 +79,7 @@ def __init__(self, sm: "StateMachine"):
     def empty(self):
         return self.external_queue.is_empty()
 
-    def put(self, trigger_data: TriggerData, internal: bool = False):
+    def put(self, trigger_data: TriggerData, internal: bool = False, _delayed: bool = False):
         """Put the trigger on the queue without blocking the caller."""
         if not self.running and not self.sm.allow_event_without_transition:
             raise TransitionNotAllowed(trigger_data.event, self.sm.configuration)
@@ -89,6 +89,13 @@ def put(self, trigger_data: TriggerData, internal: bool = False):
         else:
             self.external_queue.put(trigger_data)
 
+        if not _delayed:
+            logger.debug(
+                "New event '%s' put on the '%s' queue",
+                trigger_data.event,
+                "internal" if internal else "external",
+            )
+
     def pop(self):
         return self.external_queue.pop()
 
@@ -268,14 +275,19 @@ def microstep(self, transitions: List[Transition], trigger_data: TriggerData):
         """Process a single set of transitions in a 'lock step'.
         This includes exiting states, executing transition content, and entering states.
         """
-        result = self._execute_transition_content(
-            transitions, trigger_data, lambda t: t.before.key
-        )
+        previous_configuration = self.sm.configuration
+        try:
+            result = self._execute_transition_content(
+                transitions, trigger_data, lambda t: t.before.key
+            )
 
-        states_to_exit = self._exit_states(transitions, trigger_data)
-        logger.debug("States to exit: %s", states_to_exit)
-        result += self._execute_transition_content(transitions, trigger_data, lambda t: t.on.key)
-        self._enter_states(transitions, trigger_data, states_to_exit)
+            states_to_exit = self._exit_states(transitions, trigger_data)
+            result += self._enter_states(
+                transitions, trigger_data, states_to_exit, previous_configuration
+            )
+        except Exception:
+            self.sm.configuration = previous_configuration
+            raise
         self._execute_transition_content(
             transitions,
             trigger_data,
@@ -291,12 +303,13 @@ def microstep(self, transitions: List[Transition], trigger_data: TriggerData):
         return result
 
     def _get_args_kwargs(
-        self, transition: Transition, trigger_data: TriggerData, set_target_as_state: bool = False
+        self, transition: Transition, trigger_data: TriggerData, target: "State | None" = None
     ):
         # TODO: Ideally this method should be called only once per microstep/transition
         event_data = EventData(trigger_data=trigger_data, transition=transition)
-        if set_target_as_state:
-            event_data.state = transition.target
+        if target:
+            event_data.state = target
+            event_data.target = target
 
         args, kwargs = event_data.args, event_data.extended_kwargs
 
@@ -319,10 +332,13 @@ def _exit_states(self, enabled_transitions: List[Transition], trigger_data: Trig
         # for state in states_to_exit:
         #     self.states_to_invoke.discard(state)
 
-        # TODO: Sort states to exit in exit order
-        # states_to_exit = sorted(states_to_exit, key=self.exit_order)
+        ordered_states = sorted(
+            states_to_exit, key=lambda x: x.source and x.source.document_order or 0, reverse=True
+        )
+        result = OrderedSet([info.source for info in ordered_states])
+        logger.debug("States to exit: %s", result)
 
-        for info in states_to_exit:
+        for info in ordered_states:
             args, kwargs = self._get_args_kwargs(info.transition, trigger_data)
 
             # # TODO: Update history
@@ -342,22 +358,28 @@ def _exit_states(self, enabled_transitions: List[Transition], trigger_data: Trig
             #     self.cancel_invoke(invocation)
 
             # Remove state from configuration
-            # self.sm.configuration -= {info.source}  # .discard(info.source)
+            if not self.sm.atomic_configuration_update:
+                self.sm.configuration -= {info.source}  # .discard(info.source)
 
-        return OrderedSet([info.source for info in states_to_exit])
+        return result
 
     def _execute_transition_content(
         self,
         enabled_transitions: List[Transition],
         trigger_data: TriggerData,
         get_key: Callable[[Transition], str],
         set_target_as_state: bool = False,
+        **kwargs_extra,
     ):
         result = []
         for transition in enabled_transitions:
+            target = transition.target if set_target_as_state else None
             args, kwargs = self._get_args_kwargs(
-                transition, trigger_data, set_target_as_state=set_target_as_state
+                transition,
+                trigger_data,
+                target=target,
             )
+            kwargs.update(kwargs_extra)
 
             result += self.sm._callbacks.call(get_key(transition), *args, **kwargs)
 
@@ -368,6 +390,7 @@ def _enter_states(
         enabled_transitions: List[Transition],
         trigger_data: TriggerData,
         states_to_exit: OrderedSet[State],
+        previous_configuration: OrderedSet[State],
     ):
         """Enter the states as determined by the given transitions."""
         states_to_enter = OrderedSet[StateTransition]()
@@ -379,29 +402,44 @@ def _enter_states(
             enabled_transitions, states_to_enter, states_for_default_entry, default_history_content
         )
 
+        ordered_states = sorted(
+            states_to_enter, key=lambda x: x.source and x.source.document_order or 0
+        )
+
         # We update the configuration atomically
-        states_targets_to_enter = OrderedSet(
-            info.target for info in states_to_enter if info.target
+        states_targets_to_enter = OrderedSet(info.target for info in ordered_states if info.target)
+
+        new_configuration = cast(
+            OrderedSet[State], (previous_configuration - states_to_exit) | states_targets_to_enter
         )
         logger.debug("States to enter: %s", states_targets_to_enter)
 
-        configuration = self.sm.configuration
-        self.sm.configuration = cast(
-            OrderedSet[State], (configuration - states_to_exit) | states_targets_to_enter
+        result = self._execute_transition_content(
+            enabled_transitions,
+            trigger_data,
+            lambda t: t.on.key,
+            previous_configuration=previous_configuration,
+            new_configuration=new_configuration,
         )
 
+        if self.sm.atomic_configuration_update:
+            self.sm.configuration = new_configuration
+
         # Sort states to enter in entry order
         # for state in sorted(states_to_enter, key=self.entry_order):   # TODO: order of states_to_enter # noqa: E501
-        for info in states_to_enter:
+        for info in ordered_states:
             target = info.target
             assert target
             transition = info.transition
             args, kwargs = self._get_args_kwargs(
-                transition, trigger_data, set_target_as_state=True
+                transition,
+                trigger_data,
+                target=target,
             )
 
             # Add state to the configuration
-            # self.sm.configuration |= {target}
+            if not self.sm.atomic_configuration_update:
+                self.sm.configuration |= {target}
 
             # TODO: Add state to states_to_invoke
             # self.states_to_invoke.add(state)
@@ -412,7 +450,7 @@ def _enter_states(
             #     state.is_first_entry = False
 
             # Execute `onentry` handlers
-            self.sm._callbacks.call(target.enter.key, *args, **kwargs)
+            on_entry_result = self.sm._callbacks.call(target.enter.key, *args, **kwargs)
 
             # Handle default initial states
             # TODO: Handle default initial states
@@ -431,15 +469,24 @@ def _enter_states(
                     parent = target.parent
                     grandparent = parent.parent
 
+                    donedata = {}
+                    for item in on_entry_result:
+                        if not item:
+                            continue
+                        donedata.update(item)
+
                     BoundEvent(
                         f"done.state.{parent.id}",
                         _sm=self.sm,
                         internal=True,
-                    ).put()
+                    ).put(donedata=donedata)
 
-                    if grandparent.parallel:
+                    if grandparent and grandparent.parallel:
                         if all(child.final for child in grandparent.states):
-                            BoundEvent(f"done.state.{parent.id}", _sm=self.sm, internal=True).put()
+                            BoundEvent(f"done.state.{parent.id}", _sm=self.sm, internal=True).put(
+                                donedata=donedata
+                            )
+        return result
 
     def compute_entry_set(
         self, transitions, states_to_enter, states_for_default_entry, default_history_content