Skip to content

Commit 487574e

Browse files
committed
refactor: normalize Configuration internals to always use OrderedSet
Introduce two boundary functions (_read_from_model / _write_to_model) that confine the None|scalar|OrderedSet trichotomy to the model edge. All other methods (add, discard, states setter, values) now operate on a uniform OrderedSet, eliminating per-method type branching. The model still receives the same denormalized values as before (None for empty, scalar for single state, OrderedSet for multiple). Fixes edge case: configuration_values now returns OrderedSet() for uninitialized config instead of OrderedSet([None]), since None is not a valid state value. Prepares the codebase for the persistence protocol work (#597). Signed-off-by: Fernando Macedo <fernando.macedo@jusbrasil.com.br>
1 parent 63d00e0 commit 487574e

4 files changed

Lines changed: 88 additions & 51 deletions

File tree

docs/async.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ the initial state:
7575
... print(list(sm.configuration_values))
7676

7777
>>> asyncio.run(show_problem())
78-
[None]
78+
[]
7979

8080
```
8181

statemachine/configuration.py

Lines changed: 48 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -55,76 +55,53 @@ def value(self) -> Any:
5555

5656
@value.setter
5757
def value(self, val: Any):
58-
self._invalidate()
59-
if val is not None and not isinstance(val, MutableSet) and val not in self._states_map:
60-
raise InvalidStateValue(val)
61-
setattr(self._model, self._state_field, val)
58+
if val is None:
59+
self._write_to_model(OrderedSet())
60+
elif isinstance(val, MutableSet):
61+
self._write_to_model(OrderedSet(val) if not isinstance(val, OrderedSet) else val)
62+
else:
63+
self._write_to_model(OrderedSet([val]))
6264

6365
@property
6466
def values(self) -> OrderedSet[Any]:
6567
"""The set of raw state values currently active."""
66-
v = self.value
67-
if isinstance(v, OrderedSet):
68-
return v
69-
return OrderedSet([v])
68+
return self._read_from_model()
7069

7170
# -- Resolved states -------------------------------------------------------
7271

7372
@property
7473
def states(self) -> "OrderedSet[State]":
7574
"""The set of currently active :class:`State` instances (cached)."""
76-
csv = self.value
77-
if self._cached is not None and self._cached_value is csv:
75+
raw = self.value
76+
if self._cached is not None and self._cached_value is raw:
7877
return self._cached
79-
if csv is None:
78+
if raw is None:
8079
return OrderedSet()
8180

82-
instance_states = self._instance_states
83-
if not isinstance(csv, MutableSet):
84-
result = OrderedSet([instance_states[self._states_map[csv].id]])
85-
else:
86-
result = OrderedSet([instance_states[self._states_map[v].id] for v in csv])
87-
81+
# Normalize inline (avoid second getattr via _read_from_model)
82+
values = raw if isinstance(raw, MutableSet) else (raw,)
83+
result = OrderedSet(self._instance_states[self._states_map[v].id] for v in values)
8884
self._cached = result
89-
self._cached_value = csv
85+
self._cached_value = raw
9086
return result
9187

9288
@states.setter
9389
def states(self, new_configuration: "OrderedSet[State]"):
94-
if len(new_configuration) == 0:
95-
self.value = None
96-
elif len(new_configuration) == 1:
97-
self.value = next(iter(new_configuration)).value
98-
else:
99-
self.value = OrderedSet(s.value for s in new_configuration)
90+
self._write_to_model(OrderedSet(s.value for s in new_configuration))
10091

10192
# -- Incremental mutation (used by the engine) -----------------------------
10293

10394
def add(self, state: "State"):
104-
"""Add *state* to the configuration, maintaining the dual representation."""
105-
csv = self.value
106-
if csv is None:
107-
self.value = state.value
108-
elif isinstance(csv, MutableSet):
109-
new = OrderedSet(csv)
110-
new.add(state.value)
111-
self.value = new
112-
else:
113-
self.value = OrderedSet([csv, state.value])
95+
"""Add *state* to the configuration."""
96+
values = self._read_from_model()
97+
values.add(state.value)
98+
self._write_to_model(values)
11499

115100
def discard(self, state: "State"):
116-
"""Remove *state* from the configuration, normalizing back to scalar."""
117-
csv = self.value
118-
if isinstance(csv, MutableSet):
119-
new = OrderedSet(v for v in csv if v != state.value)
120-
if len(new) == 0:
121-
self.value = None
122-
elif len(new) == 1:
123-
self.value = next(iter(new))
124-
else:
125-
self.value = new
126-
elif csv == state.value:
127-
self.value = None
101+
"""Remove *state* from the configuration."""
102+
values = self._read_from_model()
103+
values.discard(state.value)
104+
self._write_to_model(values)
128105

129106
# -- Deprecated v2 compat --------------------------------------------------
130107

@@ -154,7 +131,31 @@ def current_state(self) -> "State | OrderedSet[State]":
154131
except KeyError as err:
155132
raise InvalidStateValue(csv) from err
156133

157-
# -- Internal --------------------------------------------------------------
134+
# -- Internal: model boundary ----------------------------------------------
135+
136+
def _read_from_model(self) -> OrderedSet:
137+
"""Normalize: model value → always ``OrderedSet``."""
138+
raw = self.value
139+
if raw is None:
140+
return OrderedSet()
141+
if isinstance(raw, OrderedSet):
142+
return raw
143+
if isinstance(raw, MutableSet):
144+
return OrderedSet(raw)
145+
return OrderedSet([raw])
146+
147+
def _write_to_model(self, values: OrderedSet):
148+
"""Denormalize: ``OrderedSet`` → ``None | scalar | OrderedSet`` for model."""
149+
self._invalidate()
150+
if len(values) == 0:
151+
raw = None
152+
elif len(values) == 1:
153+
raw = next(iter(values))
154+
else:
155+
raw = values
156+
if raw is not None and not isinstance(raw, MutableSet) and raw not in self._states_map:
157+
raise InvalidStateValue(raw)
158+
setattr(self._model, self._state_field, raw)
158159

159160
def _invalidate(self):
160161
self._cached = None

tests/test_api_contract.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,7 @@ async def test_set_none_clears_configuration(sm_runner):
243243

244244
assert model.state is None
245245
assert sm.current_state_value is None
246-
assert sm.configuration_values == OrderedSet([None])
246+
assert sm.configuration_values == OrderedSet()
247247
assert sm.configuration == OrderedSet()
248248

249249

@@ -269,10 +269,10 @@ async def test_uninitialized_then_activated(sc_class, expected_ids):
269269
model = Model()
270270
sm = sc_class(model=model, listeners=[_AsyncListener()])
271271

272-
# Before activation: model.state is None, configuration_values wraps it
272+
# Before activation: all APIs reflect empty configuration
273273
assert model.state is None
274274
assert sm.current_state_value is None
275-
assert sm.configuration_values == OrderedSet([None])
275+
assert sm.configuration_values == OrderedSet()
276276
assert sm.configuration == OrderedSet()
277277

278278
# After activation: full contract holds

tests/test_configuration.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,42 @@ def test_set_multi_element_configuration(self):
4141
assert sm.current_state_value == OrderedSet([ParallelSM.s1.value, ParallelSM.s2.value])
4242

4343

44+
class TestConfigurationValueSetter:
45+
def test_set_value_none_writes_none_to_model(self):
46+
sm = ParallelSM()
47+
assert sm.current_state_value is not None
48+
49+
sm.current_state_value = None
50+
assert sm.current_state_value is None
51+
assert sm.configuration_values == OrderedSet()
52+
53+
def test_set_value_plain_set_coerces_to_ordered_set(self):
54+
sm = ParallelSM()
55+
s1_val = ParallelSM.s1.value
56+
s2_val = ParallelSM.s2.value
57+
58+
# Assign a plain set (MutableSet but not OrderedSet)
59+
sm.current_state_value = {s1_val, s2_val}
60+
# Model should store an OrderedSet (denormalized back to it)
61+
assert isinstance(sm.current_state_value, OrderedSet)
62+
assert sm.current_state_value == OrderedSet([s1_val, s2_val])
63+
64+
65+
class TestReadFromModelNonOrderedSet:
66+
def test_read_from_model_coerces_plain_set(self):
67+
"""When the model stores a plain set, _read_from_model coerces it."""
68+
sm = ParallelSM()
69+
s1_val = ParallelSM.s1.value
70+
s2_val = ParallelSM.s2.value
71+
72+
# Bypass the value setter to place a plain set on the model
73+
setattr(sm._config._model, sm._config._state_field, {s1_val, s2_val})
74+
75+
values = sm._config._read_from_model()
76+
assert isinstance(values, OrderedSet)
77+
assert values == OrderedSet([s1_val, s2_val])
78+
79+
4480
class TestConfigurationDiscard:
4581
def test_discard_nonmatching_scalar(self):
4682
sm = ParallelSM()

0 commit comments

Comments
 (0)