Skip to content

Commit 2402b58

Browse files
BroadleafCommerce/QA#1392 - Ensure that rollback handlers are segregated to the context of a single workflow and child workflows cannot trigger rollback changes in parents
1 parent 7a41f39 commit 2402b58

File tree

5 files changed

+213
-12
lines changed

5 files changed

+213
-12
lines changed

core/broadleaf-framework/src/main/java/org/broadleafcommerce/core/workflow/SequenceProcessor.java

+6-8
Original file line numberDiff line numberDiff line change
@@ -53,13 +53,12 @@ public ProcessContext<?> doActivities(Object seedData) throws WorkflowException
5353
throw new IllegalStateException("Unable to find an instance of ActivityStateManager registered under bean id blActivityStateManager");
5454
}
5555
ProcessContext<?> context = null;
56-
RollbackStateLocal rollbackStateLocal = RollbackStateLocal.getRollbackStateLocal();
57-
if (rollbackStateLocal == null) {
58-
rollbackStateLocal = new RollbackStateLocal();
59-
rollbackStateLocal.setThreadId(String.valueOf(Thread.currentThread().getId()));
60-
rollbackStateLocal.setWorkflowId(getBeanName());
61-
RollbackStateLocal.setRollbackStateLocal(rollbackStateLocal);
62-
}
56+
57+
RollbackStateLocal rollbackStateLocal = new RollbackStateLocal();
58+
rollbackStateLocal.setThreadId(String.valueOf(Thread.currentThread().getId()));
59+
rollbackStateLocal.setWorkflowId(getBeanName());
60+
RollbackStateLocal.setRollbackStateLocal(rollbackStateLocal);
61+
6362
try {
6463
//retrieve injected by Spring
6564
List<Activity<ProcessContext<?>>> activities = getActivities();
@@ -119,7 +118,6 @@ public ProcessContext<?> doActivities(Object seedData) throws WorkflowException
119118
rollbackStateLocal = RollbackStateLocal.getRollbackStateLocal();
120119
if (rollbackStateLocal != null && rollbackStateLocal.getWorkflowId().equals(getBeanName())) {
121120
activityStateManager.clearAllState();
122-
RollbackStateLocal.setRollbackStateLocal(null);
123121
}
124122
}
125123
LOG.debug(getBeanName() + " processor is done.");

core/broadleaf-framework/src/main/java/org/broadleafcommerce/core/workflow/state/ActivityStateManagerImpl.java

+1
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ public void init() {
5353
public void clearAllState() {
5454
RollbackStateLocal rollbackStateLocal = getRollbackStateLocal();
5555
stateMap.remove(rollbackStateLocal.getThreadId() + "_" + rollbackStateLocal.getWorkflowId());
56+
RollbackStateLocal.clearRollbackStateLocal();
5657
}
5758

5859
@Override

core/broadleaf-framework/src/main/java/org/broadleafcommerce/core/workflow/state/RollbackStateLocal.java

+11-3
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@
2121

2222
import org.broadleafcommerce.common.classloader.release.ThreadLocalManager;
2323

24+
import java.util.Stack;
25+
2426
/**
2527
* Handles the identification of the outermost workflow and the current thread so that the StateManager can
2628
* operate on the appropriate RollbackHandlers.
@@ -29,14 +31,20 @@
2931
*/
3032
public class RollbackStateLocal {
3133

32-
private static final ThreadLocal<RollbackStateLocal> THREAD_LOCAL = ThreadLocalManager.createThreadLocal(RollbackStateLocal.class, false);
34+
private static final ThreadLocal<Stack> THREAD_LOCAL = ThreadLocalManager.createThreadLocal(Stack.class, true);
3335

3436
public static RollbackStateLocal getRollbackStateLocal() {
35-
return THREAD_LOCAL.get();
37+
return (RollbackStateLocal) THREAD_LOCAL.get().peek();
3638
}
3739

3840
public static void setRollbackStateLocal(RollbackStateLocal rollbackStateLocal) {
39-
THREAD_LOCAL.set(rollbackStateLocal);
41+
Stack localState = THREAD_LOCAL.get();
42+
localState.push(rollbackStateLocal);
43+
}
44+
45+
public static void clearRollbackStateLocal() {
46+
Stack localState = THREAD_LOCAL.get();
47+
localState.pop();
4048
}
4149

4250
private String threadId;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
/*
2+
* #%L
3+
* BroadleafCommerce Integration
4+
* %%
5+
* Copyright (C) 2009 - 2016 Broadleaf Commerce
6+
* %%
7+
* Licensed under the Apache License, Version 2.0 (the "License");
8+
* you may not use this file except in compliance with the License.
9+
* You may obtain a copy of the License at
10+
*
11+
* http://www.apache.org/licenses/LICENSE-2.0
12+
*
13+
* Unless required by applicable law or agreed to in writing, software
14+
* distributed under the License is distributed on an "AS IS" BASIS,
15+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
16+
* See the License for the specific language governing permissions and
17+
* limitations under the License.
18+
* #L%
19+
*/
20+
/**
21+
*
22+
*/
23+
package org.broadleafcommerce.common.workflow;
24+
25+
import org.broadleafcommerce.core.workflow.Activity;
26+
import org.broadleafcommerce.core.workflow.BaseActivity;
27+
import org.broadleafcommerce.core.workflow.DefaultProcessContextImpl;
28+
import org.broadleafcommerce.core.workflow.ProcessContext;
29+
import org.broadleafcommerce.core.workflow.ProcessContextFactory;
30+
import org.broadleafcommerce.core.workflow.SequenceProcessor;
31+
import org.broadleafcommerce.core.workflow.WorkflowException;
32+
import org.broadleafcommerce.core.workflow.state.RollbackFailureException;
33+
import org.broadleafcommerce.core.workflow.state.RollbackHandler;
34+
import org.broadleafcommerce.test.BaseTest;
35+
import org.testng.Assert;
36+
import org.testng.annotations.Test;
37+
38+
import java.util.ArrayList;
39+
import java.util.Arrays;
40+
import java.util.List;
41+
import java.util.Map;
42+
43+
import javax.annotation.Resource;
44+
45+
/**
46+
* Ensures that activities are rolled back in the correct order
47+
*
48+
* @author Phillip Verheyden (phillipuniverse)
49+
*/
50+
public class RollbackTest extends BaseTest {
51+
52+
@Resource(name = "testRollbackWorkflow")
53+
protected SequenceProcessor testRollbackWorkflow;
54+
55+
@Test
56+
public void testRollbackOrder() {
57+
List<String> results = new ArrayList<String>();
58+
boolean exceptionThrown = false;
59+
try {
60+
testRollbackWorkflow.doActivities(results);
61+
} catch (WorkflowException e) {
62+
exceptionThrown = true;
63+
}
64+
65+
List<String> expected = Arrays.asList("Activity1",
66+
"Activity2",
67+
"ActivityA",
68+
"RollbackActivityA",
69+
"NestedActivityException",
70+
"RollbackActivity2",
71+
"RollbackActivity1");
72+
Assert.assertTrue(exceptionThrown);
73+
Assert.assertEquals(results, expected, "Rollback occurred out of order");
74+
}
75+
76+
public static class SimpleActivity extends BaseActivity<ProcessContext<List<String>>> {
77+
78+
protected String name;
79+
80+
public SimpleActivity(String name) {
81+
this.name = name;
82+
}
83+
84+
public String getName() {
85+
return name;
86+
}
87+
88+
@Override
89+
public ProcessContext<List<String>> execute(ProcessContext<List<String>> context) throws Exception {
90+
context.getSeedData().add(name);
91+
return context;
92+
}
93+
94+
@Override
95+
public boolean getAutomaticallyRegisterRollbackHandler() {
96+
return true;
97+
}
98+
99+
}
100+
101+
public static class SimpleRollbackHandler implements RollbackHandler<List<String>> {
102+
103+
@Override
104+
public void rollbackState(Activity<? extends ProcessContext<List<String>>> activity, ProcessContext<List<String>> processContext, Map<String, Object> stateConfiguration) throws RollbackFailureException {
105+
processContext.getSeedData().add("Rollback" + ((SimpleActivity) activity).getName());
106+
}
107+
108+
}
109+
110+
public static class NestedActivity extends BaseActivity<ProcessContext<List<String>>> {
111+
112+
protected SequenceProcessor workflow;
113+
114+
public NestedActivity(SequenceProcessor workflow) {
115+
this.workflow = workflow;
116+
}
117+
118+
@Override
119+
public ProcessContext<List<String>> execute(ProcessContext<List<String>> context) throws Exception {
120+
try {
121+
workflow.doActivities(context.getSeedData());
122+
} catch (WorkflowException e) {
123+
context.getSeedData().add("NestedActivityException");
124+
throw e;
125+
}
126+
return context;
127+
}
128+
129+
}
130+
131+
public static class ExceptionActivity extends BaseActivity<ProcessContext<List<String>>> {
132+
133+
@Override
134+
public ProcessContext<List<String>> execute(ProcessContext<List<String>> context) throws Exception {
135+
throw new RuntimeException();
136+
}
137+
138+
}
139+
140+
public static class DummyProcessContextFactory implements ProcessContextFactory<Object, Object> {
141+
142+
@Override
143+
public ProcessContext<Object> createContext(Object preSeedData) throws WorkflowException {
144+
ProcessContext<Object> context = new DefaultProcessContextImpl<>();
145+
context.setSeedData(preSeedData);
146+
return context;
147+
}
148+
149+
}
150+
}

integration/src/test/resources/bl-applicationContext-test.xml

+45-1
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,51 @@
112112
</property>
113113
<property name="defaultErrorHandler" ref="blDefaultErrorHandler"/>
114114
</bean>
115-
115+
116+
<bean id="testRollbackWorkflow" class="org.broadleafcommerce.core.workflow.SequenceProcessor">
117+
<property name="processContextFactory">
118+
<bean class="org.broadleafcommerce.common.workflow.RollbackTest.DummyProcessContextFactory"/>
119+
</property>
120+
<property name="activities">
121+
<list>
122+
<bean class="org.broadleafcommerce.common.workflow.RollbackTest.SimpleActivity">
123+
<constructor-arg value="Activity1"></constructor-arg>
124+
<property name="rollbackHandler">
125+
<bean class="org.broadleafcommerce.common.workflow.RollbackTest.SimpleRollbackHandler" />
126+
</property>
127+
</bean>
128+
<bean class="org.broadleafcommerce.common.workflow.RollbackTest.SimpleActivity">
129+
<constructor-arg value="Activity2"></constructor-arg>
130+
<property name="rollbackHandler">
131+
<bean class="org.broadleafcommerce.common.workflow.RollbackTest.SimpleRollbackHandler" />
132+
</property>
133+
</bean>
134+
<bean class="org.broadleafcommerce.common.workflow.RollbackTest.NestedActivity">
135+
<constructor-arg name="workflow">
136+
<bean class="org.broadleafcommerce.core.workflow.SequenceProcessor">
137+
<property name="processContextFactory">
138+
<bean class="org.broadleafcommerce.common.workflow.RollbackTest.DummyProcessContextFactory"/>
139+
</property>
140+
<property name="activities">
141+
<list>
142+
<bean class="org.broadleafcommerce.common.workflow.RollbackTest.SimpleActivity">
143+
<constructor-arg value="ActivityA"></constructor-arg>
144+
<property name="rollbackHandler">
145+
<bean class="org.broadleafcommerce.common.workflow.RollbackTest.SimpleRollbackHandler" />
146+
</property>
147+
</bean>
148+
<bean class="org.broadleafcommerce.common.workflow.RollbackTest.ExceptionActivity" />
149+
</list>
150+
</property>
151+
<property name="defaultErrorHandler" ref="blSilentErrorHandler"/>
152+
</bean>
153+
</constructor-arg>
154+
</bean>
155+
</list>
156+
</property>
157+
<property name="defaultErrorHandler" ref="blSilentErrorHandler"/>
158+
</bean>
159+
116160
<bean id="blMergedCacheConfigLocations" class="org.springframework.beans.factory.config.ListFactoryBean">
117161
<property name="sourceList">
118162
<list>

0 commit comments

Comments
 (0)