diff --git a/statemachine/configuration.py b/statemachine/configuration.py index 87bdb265..0a293658 100644 --- a/statemachine/configuration.py +++ b/statemachine/configuration.py @@ -33,7 +33,7 @@ class Configuration: def __init__( self, - instance_states: "Mapping[str, State]", + instance_states: "Mapping[Any, State]", model: Any, state_field: str, states_map: "dict[Any, State]", @@ -84,7 +84,7 @@ def states(self) -> "OrderedSet[State]": # Normalize inline (avoid second getattr via _read_from_model) values = raw if isinstance(raw, MutableSet) else (raw,) - result = OrderedSet(self._instance_states[self._states_map[v].id] for v in values) + result = OrderedSet(self._instance_states[v] for v in values) self._cached = result self._cached_value = raw return result diff --git a/statemachine/statemachine.py b/statemachine/statemachine.py index 2d8839cc..5277c4fc 100644 --- a/statemachine/statemachine.py +++ b/statemachine/statemachine.py @@ -204,11 +204,11 @@ def _resolve_class_listeners(self, **kwargs: Any) -> list[object]: def _build_configuration(self) -> Configuration: """Create InstanceState entries and return a new Configuration.""" - instance_states: dict[str, Any] = {} + instance_states: dict[Any, Any] = {} events = self.__class__._events for state in self.states_map.values(): ist = InstanceState(state, self) - instance_states[state.id] = ist + instance_states[state.value] = ist if state.id not in events: vars(self)[state.id] = ist return Configuration( diff --git a/tests/test_configuration.py b/tests/test_configuration.py index 07f13ea6..e78bcdc4 100644 --- a/tests/test_configuration.py +++ b/tests/test_configuration.py @@ -215,3 +215,95 @@ async def test_parallel_with_serializing_model_both_engines(self, sm_runner): await sm_runner.send(sm, "move_a") assert "a2" in sm.configuration_values assert len(sm.configuration_values) == 5 + + +# --------------------------------------------------------------------------- +# Regression tests for issue #624: sibling compound states whose children +# share the same ``id`` (e.g. ``a``, ``b``) but use distinct ``value=`` +# identifiers must not collapse in ``_instance_states``. +# +# Before the fix, ``_instance_states`` was keyed by ``state.id`` — so two +# states with id ``"a"`` overwrote each other, and event dispatch raised +# ``TransitionNotAllowed`` because the cached configuration resolved to the +# wrong ``State`` instance. Phase A re-keyed by ``state.value`` (which is +# globally unique, since it's the canonical identifier used by +# ``states_map``). +# --------------------------------------------------------------------------- + + +class SiblingCompoundChart(StateChart): + """Canonical repro from issue #624. + + Two sibling ``State.Compound`` regions, each containing children named + ``a`` and ``b``. The ``value=`` strings (e.g. ``"g1__a"``) make each + state globally unique despite the shared local id. + """ + + allow_event_without_transition = False + + class g1(State.Compound): + a = State(initial=True, value="g1__a") + b = State(final=True, value="g1__b") + to_b = a.to(b, event="g1__b") + + class g2(State.Compound): + a = State(initial=True, value="g2__a") + b = State(final=True, value="g2__b") + to_b = a.to(b, event="g2__b") + + advance = g1.to(g2) + + +class TestSiblingCompoundIdCollision: + """Regression: sibling compounds with same-named children (#624).""" + + async def test_dispatch_to_g1_inner_state_both_engines(self, sm_runner): + sm = await sm_runner.start(SiblingCompoundChart) + + # Initial configuration: g1 compound active, g1__a as initial child. + assert "g1__a" in sm.configuration_values + assert "g1" in sm.configuration_values + + # Before the fix, this raised TransitionNotAllowed because the + # _instance_states key "a" was overwritten by g2's "a". + await sm_runner.send(sm, "g1__b") + + assert "g1__b" in sm.configuration_values + assert "g1__a" not in sm.configuration_values + + async def test_dispatch_to_g2_inner_state_both_engines(self, sm_runner): + sm = await sm_runner.start(SiblingCompoundChart) + + # Move from g1 to g2 first. + await sm_runner.send(sm, "advance") + assert "g2__a" in sm.configuration_values + assert "g2" in sm.configuration_values + + # Same scenario as g1, but in the second sibling compound. + await sm_runner.send(sm, "g2__b") + + assert "g2__b" in sm.configuration_values + assert "g2__a" not in sm.configuration_values + assert "g1__b" not in sm.configuration_values + + def test_instance_states_keyed_by_value_no_collision(self): + """White-box invariant: ``_instance_states`` must not collapse keys. + + Before the fix for #624, ``_instance_states`` was keyed by ``state.id`` + (the Python attribute name) — so sibling compounds with same-named + children (both ``a``) collapsed to a single entry, leaving the dict + with fewer entries than ``states_map``. This test pins the post-fix + invariant: one entry per registered state, keyed by ``state.value``. + + The dispatch tests above prove the correct ``InstanceState`` is + wrapped at each key (a wrong wrapping would break dispatch). + """ + sm = SiblingCompoundChart() + + instance_states = sm._config._instance_states # type: ignore[attr-defined] + + # No key collapse: one entry per registered state. + assert len(instance_states) == len(sm.states_map) + + # Keyed by the canonical identifier (state.value), matching states_map. + assert set(instance_states.keys()) == set(sm.states_map.keys())