Skip to content

Commit ab31c94

Browse files
jvalkealjvalkeal
committed
Fix state holder usage
- In AbstractStateMachineFactory change used holder map to list so that same states don't get lost because of used key. - Fixes #416
1 parent 5630769 commit ab31c94

File tree

2 files changed

+104
-13
lines changed

2 files changed

+104
-13
lines changed

‎spring-statemachine-core/src/main/java/org/springframework/statemachine/config/AbstractStateMachineFactory.java

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ public StateMachine<S, E> getStateMachine(UUID uuid, String machineId) {
180180
Stack<MachineStackItem<S, E>> regionStack = new Stack<MachineStackItem<S, E>>();
181181
Stack<StateData<S, E>> stateStack = new Stack<StateData<S, E>>();
182182
Map<Object, StateMachine<S, E>> machineMap = new HashMap<Object, StateMachine<S,E>>();
183-
Map<S, StateHolder<S, E>> holderMap = new HashMap<S, StateHolder<S, E>>();
183+
List<HolderListItem<S, E>> holderList = new ArrayList<>();
184184

185185
Iterator<Node<StateData<S, E>>> iterator = buildStateDataIterator(stateMachineModel);
186186
while (iterator.hasNext()) {
@@ -216,7 +216,7 @@ public StateMachine<S, E> getStateMachine(UUID uuid, String machineId) {
216216

217217
if (initialCount > 1) {
218218
for (Collection<StateData<S, E>> regionStateDatas : regionsStateDatas) {
219-
machine = buildMachine(machineMap, stateMap, holderMap, regionStateDatas, transitionsData, resolveBeanFactory(stateMachineModel),
219+
machine = buildMachine(machineMap, stateMap, holderList, regionStateDatas, transitionsData, resolveBeanFactory(stateMachineModel),
220220
contextEvents, defaultExtendedState, stateMachineModel.getTransitionsData(), resolveTaskExecutor(stateMachineModel),
221221
resolveTaskScheduler(stateMachineModel), machineId, null, stateMachineModel);
222222
regionStack.push(new MachineStackItem<S, E>(machine));
@@ -245,7 +245,7 @@ null, defaultExtendedState, null, contextEvents, resolveBeanFactory(stateMachine
245245
machine = m;
246246
}
247247
} else {
248-
machine = buildMachine(machineMap, stateMap, holderMap, stateDatas, transitionsData, resolveBeanFactory(stateMachineModel), contextEvents,
248+
machine = buildMachine(machineMap, stateMap, holderList, stateDatas, transitionsData, resolveBeanFactory(stateMachineModel), contextEvents,
249249
defaultExtendedState, stateMachineModel.getTransitionsData(), resolveTaskExecutor(stateMachineModel), resolveTaskScheduler(stateMachineModel),
250250
machineId, uuid, stateMachineModel);
251251
if (peek.isInitial() || (!peek.isInitial() && !machineMap.containsKey(peek.getParent()))) {
@@ -321,8 +321,8 @@ public void apply(StateMachineAccess<S, E> function) {
321321

322322
// go through holders and fix state references which
323323
// were not known at a time holder was created
324-
for (Entry<S, StateHolder<S, E>> holder : holderMap.entrySet()) {
325-
holder.getValue().setState(stateMap.get(holder.getKey()));
324+
for (HolderListItem<S, E> holderItem : holderList) {
325+
holderItem.value.setState(stateMap.get(holderItem.key));
326326
}
327327

328328
// set parent machines for each built machine
@@ -519,7 +519,7 @@ private Collection<TransitionData<S, E>> resolveTransitionData2(Collection<Trans
519519

520520
@SuppressWarnings("unchecked")
521521
private StateMachine<S, E> buildMachine(Map<Object, StateMachine<S, E>> machineMap, Map<S, State<S, E>> stateMap,
522-
Map<S, StateHolder<S, E>> holderMap, Collection<StateData<S, E>> stateDatas, Collection<TransitionData<S, E>> transitionsData,
522+
List<HolderListItem<S, E>> holderList, Collection<StateData<S, E>> stateDatas, Collection<TransitionData<S, E>> transitionsData,
523523
BeanFactory beanFactory, Boolean contextEvents, DefaultExtendedState defaultExtendedState,
524524
TransitionsData<S, E> stateMachineTransitions, TaskExecutor taskExecutor, TaskScheduler taskScheduler, String machineId,
525525
UUID uuid, StateMachineModel<S, E> stateMachineModel) {
@@ -614,7 +614,7 @@ private StateMachine<S, E> buildMachine(Map<Object, StateMachine<S, E>> machineM
614614
StateHolder<S, E> defaultStateHolder = new StateHolder<S, E>(defaultState);
615615
StateHolder<S, E> containingStateHolder = new StateHolder<S, E>(stateMap.get(stateData.getParent()));
616616
if (containingStateHolder.getState() == null) {
617-
holderMap.put((S)stateData.getParent(), containingStateHolder);
617+
holderList.add(new HolderListItem<S, E>((S)stateData.getParent(), containingStateHolder));
618618
}
619619
PseudoState<S, E> pseudoState = new HistoryPseudoState<S, E>(PseudoStateKind.HISTORY_SHALLOW, defaultStateHolder, containingStateHolder);
620620
state = buildStateInternal(stateData.getState(), stateData.getDeferred(), stateData.getEntryActions(),
@@ -634,7 +634,7 @@ private StateMachine<S, E> buildMachine(Map<Object, StateMachine<S, E>> machineM
634634
StateHolder<S, E> defaultStateHolder = new StateHolder<S, E>(defaultState);
635635
StateHolder<S, E> containingStateHolder = new StateHolder<S, E>(stateMap.get(stateData.getParent()));
636636
if (containingStateHolder.getState() == null) {
637-
holderMap.put((S)stateData.getParent(), containingStateHolder);
637+
holderList.add(new HolderListItem<S, E>((S)stateData.getParent(), containingStateHolder));
638638
}
639639
PseudoState<S, E> pseudoState = new HistoryPseudoState<S, E>(PseudoStateKind.HISTORY_DEEP, defaultStateHolder, containingStateHolder);
640640
state = buildStateInternal(stateData.getState(), stateData.getDeferred(), stateData.getEntryActions(),
@@ -651,7 +651,7 @@ private StateMachine<S, E> buildMachine(Map<Object, StateMachine<S, E>> machineM
651651
for (ChoiceData<S, E> c : list) {
652652
StateHolder<S, E> holder = new StateHolder<S, E>(stateMap.get(c.getTarget()));
653653
if (holder.getState() == null) {
654-
holderMap.put(c.getTarget(), holder);
654+
holderList.add(new HolderListItem<S, E>(c.getTarget(), holder));
655655
}
656656
choices.add(new ChoiceStateData<S, E>(holder, c.getGuard(), c.getActions()));
657657
}
@@ -667,7 +667,7 @@ private StateMachine<S, E> buildMachine(Map<Object, StateMachine<S, E>> machineM
667667
for (JunctionData<S, E> c : list) {
668668
StateHolder<S, E> holder = new StateHolder<S, E>(stateMap.get(c.getTarget()));
669669
if (holder.getState() == null) {
670-
holderMap.put(c.getTarget(), holder);
670+
holderList.add(new HolderListItem<S, E>(c.getTarget(), holder));
671671
}
672672
junctions.add(new JunctionStateData<S, E>(holder, c.getGuard(), c.getActions()));
673673
}
@@ -696,7 +696,7 @@ private StateMachine<S, E> buildMachine(Map<Object, StateMachine<S, E>> machineM
696696
if (s.equals(entry.getSource())) {
697697
StateHolder<S, E> holder = new StateHolder<S, E>(stateMap.get(entry.getTarget()));
698698
if (holder.getState() == null) {
699-
holderMap.put(entry.getTarget(), holder);
699+
holderList.add(new HolderListItem<S, E>(entry.getTarget(), holder));
700700
}
701701
PseudoState<S, E> pseudoState = new ExitPseudoState<S, E>(holder);
702702
state = buildStateInternal(stateData.getState(), stateData.getDeferred(), stateData.getEntryActions(),
@@ -751,7 +751,7 @@ private StateMachine<S, E> buildMachine(Map<Object, StateMachine<S, E>> machineM
751751
if (tt.getSource() == s) {
752752
StateHolder<S, E> holder = new StateHolder<S, E>(stateMap.get(tt.getTarget()));
753753
if (holder.getState() == null) {
754-
holderMap.put(tt.getTarget(), holder);
754+
holderList.add(new HolderListItem<S, E>(tt.getTarget(), holder));
755755
}
756756
joinTargets.add(new JoinStateData<S, E>(holder, tt.getGuard()));
757757
}
@@ -891,4 +891,14 @@ public void stateMachineStarted(StateMachine<S, E> stateMachine) {
891891
latch.countDown();
892892
}
893893
}
894+
895+
private static class HolderListItem<S, E> {
896+
S key;
897+
StateHolder<S, E> value;
898+
899+
public HolderListItem(S key, StateHolder<S, E> value) {
900+
this.key = key;
901+
this.value = value;
902+
}
903+
}
894904
}

‎spring-statemachine-core/src/test/java/org/springframework/statemachine/state/ExitEntryStateTests.java

Lines changed: 82 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2016 the original author or authors.
2+
* Copyright 2016-2017 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -48,6 +48,29 @@ public void testSimpleEntryExit() {
4848
assertThat(machine.getState().getIds(), contains("S4"));
4949
}
5050

51+
@SuppressWarnings("unchecked")
52+
@Test
53+
public void testMultipleExitsToSameState() {
54+
context.register(Config2.class);
55+
context.refresh();
56+
StateMachine<String, String> machine =
57+
context.getBean(StateMachineSystemConstants.DEFAULT_ID_STATEMACHINE, StateMachine.class);
58+
assertThat(machine, notNullValue());
59+
machine.start();
60+
61+
assertThat(machine.getState().getIds(), contains("S1"));
62+
machine.sendEvent("E1");
63+
assertThat(machine.getState().getIds(), contains("S2", "S22"));
64+
machine.sendEvent("EXIT2");
65+
assertThat(machine.getState().getIds(), contains("S1"));
66+
67+
machine.sendEvent("E2");
68+
assertThat(machine.getState().getIds(), contains("S3", "S32"));
69+
machine.sendEvent("EXIT3");
70+
assertThat(machine.getState().getIds(), contains("S1"));
71+
72+
}
73+
5174
@Configuration
5275
@EnableStateMachine
5376
static class Config1 extends StateMachineConfigurerAdapter<String, String> {
@@ -116,6 +139,64 @@ public void configure(StateMachineTransitionConfigurer<String, String> transitio
116139
}
117140
}
118141

142+
@Configuration
143+
@EnableStateMachine
144+
static class Config2 extends StateMachineConfigurerAdapter<String, String> {
145+
146+
@Override
147+
public void configure(StateMachineStateConfigurer<String, String> states) throws Exception {
148+
states
149+
.withStates()
150+
.initial("S1")
151+
.state("S2")
152+
.state("S3")
153+
.and()
154+
.withStates()
155+
.parent("S2")
156+
.initial("S21")
157+
.exit("S2EXIT")
158+
.state("S22")
159+
.and()
160+
.withStates()
161+
.parent("S3")
162+
.initial("S31")
163+
.exit("S3EXIT")
164+
.state("S32");
165+
}
166+
167+
@Override
168+
public void configure(StateMachineTransitionConfigurer<String, String> transitions) throws Exception {
169+
transitions
170+
.withExternal()
171+
.source("S1").target("S2")
172+
.event("E1")
173+
.and()
174+
.withExternal()
175+
.source("S1").target("S3")
176+
.event("E2")
177+
.and()
178+
.withExternal()
179+
.source("S21").target("S22")
180+
.and()
181+
.withExternal()
182+
.source("S31").target("S32")
183+
.and()
184+
.withExternal()
185+
.source("S22").target("S2EXIT")
186+
.event("EXIT2")
187+
.and()
188+
.withExternal()
189+
.source("S32").target("S3EXIT")
190+
.event("EXIT3")
191+
.and()
192+
.withExit()
193+
.source("S2EXIT").target("S1")
194+
.and()
195+
.withExit()
196+
.source("S3EXIT").target("S1");
197+
}
198+
}
199+
119200

120201
@Override
121202
protected AnnotationConfigApplicationContext buildContext() {

0 commit comments

Comments
 (0)