Skip to content

Commit e15ef09

Browse files
filiphrfiliphr
committed
Reverse iteration order for deletion methods in composite history managers
Deletion methods now iterate history managers in reverse order so that managers added later can read data (e.g. entity links) before earlier managers delete it. Affected methods: - CompositeHistoryManager: recordProcessInstanceDeleted, recordDeleteHistoricProcessInstancesByProcessDefinitionId, recordBulkDeleteProcessInstances, recordHistoricTaskDeleted - CompositeCmmnHistoryManager: recordHistoricCaseInstanceDeleted, recordBulkDeleteHistoricCaseInstances, recordHistoricTaskDeleted
1 parent c07ebd8 commit e15ef09

File tree

4 files changed

+106
-25
lines changed

4 files changed

+106
-25
lines changed

‎modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/history/CompositeCmmnHistoryManager.java‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
import java.util.ArrayList;
1616
import java.util.Collection;
1717
import java.util.Date;
18+
import java.util.List;
19+
import java.util.ListIterator;
1820

1921
import org.flowable.cmmn.api.repository.CaseDefinition;
2022
import org.flowable.cmmn.engine.impl.persistence.entity.CaseInstanceEntity;
@@ -32,7 +34,7 @@
3234
*/
3335
public class CompositeCmmnHistoryManager implements CmmnHistoryManager {
3436

35-
protected final Collection<CmmnHistoryManager> historyManagers;
37+
protected final List<CmmnHistoryManager> historyManagers;
3638

3739
public CompositeCmmnHistoryManager(Collection<CmmnHistoryManager> historyManagers) {
3840
this.historyManagers = new ArrayList<>(historyManagers);
@@ -90,15 +92,23 @@ public void recordMilestoneReached(MilestoneInstanceEntity milestoneInstanceEnti
9092

9193
@Override
9294
public void recordHistoricCaseInstanceDeleted(String caseInstanceId, String tenantId) {
93-
for (CmmnHistoryManager historyManager : historyManagers) {
94-
historyManager.recordHistoricCaseInstanceDeleted(caseInstanceId, tenantId);
95+
// Reverse order: managers added later may need to read data (e.g. entity links)
96+
// that earlier managers delete. Processing deletions in reverse ensures all
97+
// managers can access the data they need before it is removed.
98+
ListIterator<CmmnHistoryManager> iterator = historyManagers.listIterator(historyManagers.size());
99+
while (iterator.hasPrevious()) {
100+
iterator.previous().recordHistoricCaseInstanceDeleted(caseInstanceId, tenantId);
95101
}
96102
}
97103

98104
@Override
99105
public void recordBulkDeleteHistoricCaseInstances(Collection<String> caseInstanceIds) {
100-
for (CmmnHistoryManager historyManager : historyManagers) {
101-
historyManager.recordBulkDeleteHistoricCaseInstances(caseInstanceIds);
106+
// Reverse order: managers added later may need to read data (e.g. entity links)
107+
// that earlier managers delete. Processing deletions in reverse ensures all
108+
// managers can access the data they need before it is removed.
109+
ListIterator<CmmnHistoryManager> iterator = historyManagers.listIterator(historyManagers.size());
110+
while (iterator.hasPrevious()) {
111+
iterator.previous().recordBulkDeleteHistoricCaseInstances(caseInstanceIds);
102112
}
103113
}
104114

@@ -181,8 +191,12 @@ public void recordTaskInfoChange(TaskEntity taskEntity, Date changeTime) {
181191

182192
@Override
183193
public void recordHistoricTaskDeleted(HistoricTaskInstance task) {
184-
for (CmmnHistoryManager historyManager : historyManagers) {
185-
historyManager.recordHistoricTaskDeleted(task);
194+
// Reverse order: managers added later may need to read data (e.g. entity links)
195+
// that earlier managers delete. Processing deletions in reverse ensures all
196+
// managers can access the data they need before it is removed.
197+
ListIterator<CmmnHistoryManager> iterator = historyManagers.listIterator(historyManagers.size());
198+
while (iterator.hasPrevious()) {
199+
iterator.previous().recordHistoricTaskDeleted(task);
186200
}
187201
}
188202

‎modules/flowable-cmmn-engine/src/test/java/org/flowable/cmmn/test/history/CompositeCmmnHistoryManagerTest.java‎

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,11 @@
1414

1515
import static org.mockito.ArgumentMatchers.eq;
1616
import static org.mockito.ArgumentMatchers.same;
17+
import static org.mockito.Mockito.inOrder;
1718
import static org.mockito.Mockito.verify;
1819

1920
import java.time.Instant;
21+
import java.util.Arrays;
2022
import java.util.Collections;
2123
import java.util.Date;
2224

@@ -32,6 +34,7 @@
3234
import org.flowable.entitylink.service.impl.persistence.entity.EntityLinkEntityImpl;
3335
import org.flowable.identitylink.service.impl.persistence.entity.IdentityLinkEntity;
3436
import org.flowable.identitylink.service.impl.persistence.entity.IdentityLinkEntityImpl;
37+
import org.flowable.task.api.history.HistoricTaskInstance;
3538
import org.flowable.task.api.history.HistoricTaskLogEntryBuilder;
3639
import org.flowable.task.service.impl.BaseHistoricTaskLogEntryBuilderImpl;
3740
import org.flowable.task.service.impl.persistence.entity.TaskEntity;
@@ -40,6 +43,7 @@
4043
import org.flowable.variable.service.impl.persistence.entity.VariableInstanceEntityImpl;
4144
import org.junit.jupiter.api.BeforeEach;
4245
import org.junit.jupiter.api.Test;
46+
import org.mockito.InOrder;
4347
import org.mockito.Mock;
4448
import org.mockito.junit.jupiter.MockitoSettings;
4549

@@ -102,11 +106,21 @@ void recordMilestoneReached() {
102106
}
103107

104108
@Test
105-
void recordHistoricCaseInstanceDeleted() {
109+
void recordHistoricCaseInstanceDeletedShouldIterateInReverseOrder() {
106110
compositeHistoryManager.recordHistoricCaseInstanceDeleted("case-id", "tenant-1");
107111

108-
verify(historyManager1).recordHistoricCaseInstanceDeleted("case-id", "tenant-1");
109-
verify(historyManager2).recordHistoricCaseInstanceDeleted("case-id", "tenant-1");
112+
InOrder inOrder = inOrder(historyManager1, historyManager2);
113+
inOrder.verify(historyManager2).recordHistoricCaseInstanceDeleted("case-id", "tenant-1");
114+
inOrder.verify(historyManager1).recordHistoricCaseInstanceDeleted("case-id", "tenant-1");
115+
}
116+
117+
@Test
118+
void recordBulkDeleteHistoricCaseInstancesShouldIterateInReverseOrder() {
119+
compositeHistoryManager.recordBulkDeleteHistoricCaseInstances(Arrays.asList("id-1", "id-2"));
120+
121+
InOrder inOrder = inOrder(historyManager1, historyManager2);
122+
inOrder.verify(historyManager2).recordBulkDeleteHistoricCaseInstances(Arrays.asList("id-1", "id-2"));
123+
inOrder.verify(historyManager1).recordBulkDeleteHistoricCaseInstances(Arrays.asList("id-1", "id-2"));
110124
}
111125

112126
@Test
@@ -213,6 +227,15 @@ void recordTaskInfoChange() {
213227
verify(historyManager2).recordTaskInfoChange(same(task), eq(changeTime));
214228
}
215229

230+
@Test
231+
void recordHistoricTaskDeletedShouldIterateInReverseOrder(@Mock HistoricTaskInstance task) {
232+
compositeHistoryManager.recordHistoricTaskDeleted(task);
233+
234+
InOrder inOrder = inOrder(historyManager1, historyManager2);
235+
inOrder.verify(historyManager2).recordHistoricTaskDeleted(same(task));
236+
inOrder.verify(historyManager1).recordHistoricTaskDeleted(same(task));
237+
}
238+
216239
@Test
217240
void recordPlanItemInstanceCreated() {
218241
PlanItemInstanceEntity planItemInstance = new PlanItemInstanceEntityImpl();

‎modules/flowable-engine/src/main/java/org/flowable/engine/impl/history/CompositeHistoryManager.java‎

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
import java.util.ArrayList;
1616
import java.util.Collection;
1717
import java.util.Date;
18+
import java.util.List;
19+
import java.util.ListIterator;
1820
import java.util.Map;
1921

2022
import org.flowable.bpmn.model.FlowElement;
@@ -35,7 +37,7 @@
3537
*/
3638
public class CompositeHistoryManager implements HistoryManager {
3739

38-
protected final Collection<HistoryManager> historyManagers;
40+
protected final List<HistoryManager> historyManagers;
3941

4042
public CompositeHistoryManager(Collection<HistoryManager> historyManagers) {
4143
this.historyManagers = new ArrayList<>(historyManagers);
@@ -108,22 +110,34 @@ public void recordProcessInstanceNameChange(ExecutionEntity processInstanceExecu
108110

109111
@Override
110112
public void recordProcessInstanceDeleted(String processInstanceId, String processDefinitionId, String processTenantId) {
111-
for (HistoryManager historyManager : historyManagers) {
112-
historyManager.recordProcessInstanceDeleted(processInstanceId, processDefinitionId, processTenantId);
113+
// Reverse order: managers added later may need to read data (e.g. entity links)
114+
// that earlier managers delete. Processing deletions in reverse ensures all
115+
// managers can access the data they need before it is removed.
116+
ListIterator<HistoryManager> iterator = historyManagers.listIterator(historyManagers.size());
117+
while (iterator.hasPrevious()) {
118+
iterator.previous().recordProcessInstanceDeleted(processInstanceId, processDefinitionId, processTenantId);
113119
}
114120
}
115121

116122
@Override
117123
public void recordDeleteHistoricProcessInstancesByProcessDefinitionId(String processDefinitionId) {
118-
for (HistoryManager historyManager : historyManagers) {
119-
historyManager.recordDeleteHistoricProcessInstancesByProcessDefinitionId(processDefinitionId);
124+
// Reverse order: managers added later may need to read data (e.g. entity links)
125+
// that earlier managers delete. Processing deletions in reverse ensures all
126+
// managers can access the data they need before it is removed.
127+
ListIterator<HistoryManager> iterator = historyManagers.listIterator(historyManagers.size());
128+
while (iterator.hasPrevious()) {
129+
iterator.previous().recordDeleteHistoricProcessInstancesByProcessDefinitionId(processDefinitionId);
120130
}
121131
}
122132

123133
@Override
124134
public void recordBulkDeleteProcessInstances(Collection<String> processInstanceIds) {
125-
for (HistoryManager historyManager : historyManagers) {
126-
historyManager.recordBulkDeleteProcessInstances(processInstanceIds);
135+
// Reverse order: managers added later may need to read data (e.g. entity links)
136+
// that earlier managers delete. Processing deletions in reverse ensures all
137+
// managers can access the data they need before it is removed.
138+
ListIterator<HistoryManager> iterator = historyManagers.listIterator(historyManagers.size());
139+
while (iterator.hasPrevious()) {
140+
iterator.previous().recordBulkDeleteProcessInstances(processInstanceIds);
127141
}
128142
}
129143

@@ -183,8 +197,12 @@ public void recordTaskInfoChange(TaskEntity taskEntity, String activityInstanceI
183197

184198
@Override
185199
public void recordHistoricTaskDeleted(HistoricTaskInstance task){
186-
for (HistoryManager historyManager : historyManagers) {
187-
historyManager.recordHistoricTaskDeleted(task);
200+
// Reverse order: managers added later may need to read data (e.g. entity links)
201+
// that earlier managers delete. Processing deletions in reverse ensures all
202+
// managers can access the data they need before it is removed.
203+
ListIterator<HistoryManager> iterator = historyManagers.listIterator(historyManagers.size());
204+
while (iterator.hasPrevious()) {
205+
iterator.previous().recordHistoricTaskDeleted(task);
188206
}
189207
}
190208

‎modules/flowable-engine/src/test/java/org/flowable/engine/test/history/CompositeHistoryManagerTest.java‎

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,13 @@
1515
import static org.assertj.core.api.Assertions.assertThat;
1616
import static org.mockito.ArgumentMatchers.eq;
1717
import static org.mockito.ArgumentMatchers.same;
18+
import static org.mockito.Mockito.inOrder;
1819
import static org.mockito.Mockito.verify;
1920
import static org.mockito.Mockito.when;
2021

2122
import java.time.Instant;
2223
import java.time.temporal.ChronoUnit;
24+
import java.util.Arrays;
2325
import java.util.Collections;
2426
import java.util.Date;
2527
import java.util.HashMap;
@@ -43,14 +45,17 @@
4345
import org.flowable.entitylink.service.impl.persistence.entity.EntityLinkEntityImpl;
4446
import org.flowable.identitylink.service.impl.persistence.entity.IdentityLinkEntity;
4547
import org.flowable.identitylink.service.impl.persistence.entity.IdentityLinkEntityImpl;
48+
import org.flowable.task.api.history.HistoricTaskInstance;
4649
import org.flowable.task.api.history.HistoricTaskLogEntryBuilder;
4750
import org.flowable.task.service.impl.BaseHistoricTaskLogEntryBuilderImpl;
51+
import org.flowable.task.service.impl.persistence.entity.HistoricTaskInstanceEntityImpl;
4852
import org.flowable.task.service.impl.persistence.entity.TaskEntity;
4953
import org.flowable.task.service.impl.persistence.entity.TaskEntityImpl;
5054
import org.flowable.variable.service.impl.persistence.entity.VariableInstanceEntity;
5155
import org.flowable.variable.service.impl.persistence.entity.VariableInstanceEntityImpl;
5256
import org.junit.jupiter.api.BeforeEach;
5357
import org.junit.jupiter.api.Test;
58+
import org.mockito.InOrder;
5459
import org.mockito.Mock;
5560
import org.mockito.junit.jupiter.MockitoSettings;
5661

@@ -165,19 +170,30 @@ void recordProcessInstanceNameChange() {
165170
}
166171

167172
@Test
168-
void recordProcessInstanceDeleted() {
173+
void recordProcessInstanceDeletedShouldIterateInReverseOrder() {
169174
compositeHistoryManager.recordProcessInstanceDeleted("instance-id", "def-id", "tenant-1");
170175

171-
verify(historyManager1).recordProcessInstanceDeleted("instance-id", "def-id", "tenant-1");
172-
verify(historyManager2).recordProcessInstanceDeleted("instance-id", "def-id", "tenant-1");
176+
InOrder inOrder = inOrder(historyManager1, historyManager2);
177+
inOrder.verify(historyManager2).recordProcessInstanceDeleted("instance-id", "def-id", "tenant-1");
178+
inOrder.verify(historyManager1).recordProcessInstanceDeleted("instance-id", "def-id", "tenant-1");
173179
}
174180

175181
@Test
176-
void recordDeleteHistoricProcessInstancesByProcessDefinitionId() {
182+
void recordDeleteHistoricProcessInstancesByProcessDefinitionIdShouldIterateInReverseOrder() {
177183
compositeHistoryManager.recordDeleteHistoricProcessInstancesByProcessDefinitionId("def-4");
178184

179-
verify(historyManager1).recordDeleteHistoricProcessInstancesByProcessDefinitionId("def-4");
180-
verify(historyManager2).recordDeleteHistoricProcessInstancesByProcessDefinitionId("def-4");
185+
InOrder inOrder = inOrder(historyManager1, historyManager2);
186+
inOrder.verify(historyManager2).recordDeleteHistoricProcessInstancesByProcessDefinitionId("def-4");
187+
inOrder.verify(historyManager1).recordDeleteHistoricProcessInstancesByProcessDefinitionId("def-4");
188+
}
189+
190+
@Test
191+
void recordBulkDeleteProcessInstancesShouldIterateInReverseOrder() {
192+
compositeHistoryManager.recordBulkDeleteProcessInstances(Arrays.asList("id-1", "id-2"));
193+
194+
InOrder inOrder = inOrder(historyManager1, historyManager2);
195+
inOrder.verify(historyManager2).recordBulkDeleteProcessInstances(Arrays.asList("id-1", "id-2"));
196+
inOrder.verify(historyManager1).recordBulkDeleteProcessInstances(Arrays.asList("id-1", "id-2"));
181197
}
182198

183199
@Test
@@ -254,6 +270,16 @@ void recordTaskInfoChange() {
254270
verify(historyManager2).recordTaskInfoChange(same(task), eq("activity"), eq(changeTime));
255271
}
256272

273+
@Test
274+
void recordHistoricTaskDeletedShouldIterateInReverseOrder() {
275+
HistoricTaskInstance task = new HistoricTaskInstanceEntityImpl();
276+
compositeHistoryManager.recordHistoricTaskDeleted(task);
277+
278+
InOrder inOrder = inOrder(historyManager1, historyManager2);
279+
inOrder.verify(historyManager2).recordHistoricTaskDeleted(same(task));
280+
inOrder.verify(historyManager1).recordHistoricTaskDeleted(same(task));
281+
}
282+
257283
@Test
258284
void recordVariableCreate() {
259285
VariableInstanceEntity variable = new VariableInstanceEntityImpl();

0 commit comments

Comments
 (0)