search

flowable / flowable-engine

A compact and highly efficient workflow and Business Process Management (BPM) platform for developers, system admins and business users.
https://www.flowable.org
Apache License 2.0
7.97k stars 2.62k forks source link

Task IdentityLinkCount is incorrectly calculated if the same identityLink is deleted multiple times in the same transaction #2736

Open paulstapleton opened 3 years ago

paulstapleton commented 3 years ago

Describe the bug The identityLinkCount field on the TaskEntityImpl can be incorrectly calculated when an identitylink on the task is deleted multiple times in the same transaction, using the method taskService.deleteUserIdentityLink(...).

Each time the method to delete the identityLink is executed then identityLinkCount on the task is reduced by one. If during the same transaction the same identityLink has already been marked for deletion, then the identityLinkCount is still reduced by 1 but no extra rows in the database are removed.

This can result in a negative identityLinkCount and causes issues when the task is completed or deleted. As a negative value for the identityLinkCount causes no identityLinks to be removed when the task is removed, and leaves an inconsistency in the database.

Note, this only occurs if the deletion occurs in the same transaction and if it is done in a separate transaction then the deletion is a no-op and does not remove 1 from identityLinkCount

This is an issue for me as I have multiple listeners which manipulate the IdentityLinks on a task. Sometimes these listeners remove the same identityLink during the same transaction and cause this error to occur.

Expected behavior If the same identityLink is deleted multiple times in the same transaction, we should either get an error from the taskService.deleteUserIdentityLink(...) or the identityLinkCount should not be reduced by 1.

Code Here is some code to reproduce the problem in Flowable 6.6.0

@FlowableTest
public class TaskIdentityLinkCountNegative {

    private Logger log = LoggerFactory.getLogger(TaskIdentityLinkCountNegative.class);

    private ProcessEngine processEngine;
    private HistoryService historyService;
    private ManagementService managementService;
    private RuntimeService runtimeService;
    private TaskService taskService;

    @BeforeEach
    void setUp(ProcessEngine processEngine) {
        this.processEngine = processEngine;
        this.historyService = processEngine.getHistoryService();
        this.managementService = processEngine.getManagementService();
        this.runtimeService = processEngine.getRuntimeService();
        this.taskService = processEngine.getTaskService();
    }

    @Test
    @Deployment(resources = {
        "TaskIdentityLinkCountNegative.testUserIdentityLinkCountIsIncorrect.bpmn20.xml"
    })
    void testUserIdentityLinkCountIsIncorrect() {

        ProcessInstance processInstance = runtimeService.startProcessInstanceByKey("my-process");
        Task task = taskService.createTaskQuery().processInstanceId(processInstance.getId()).singleResult();
        String taskId = task.getId();

        taskService.addUserIdentityLink(taskId, "user1", IdentityLinkType.PARTICIPANT);
        taskService.addUserIdentityLink(taskId, "user2", IdentityLinkType.PARTICIPANT);

        managementService.executeCommand(command -> {
            taskService.deleteUserIdentityLink(taskId, "user1", IdentityLinkType.PARTICIPANT);
            taskService.deleteUserIdentityLink(taskId, "user1", IdentityLinkType.PARTICIPANT);
            taskService.deleteUserIdentityLink(taskId, "user1", IdentityLinkType.PARTICIPANT);
            return null;
        });

        CountingTaskEntity countingTaskEntity = (CountingTaskEntity) taskService.createTaskQuery().processInstanceId(processInstance.getId()).singleResult();

        log.info("IdentityLinkCount is {}", countingTaskEntity.getIdentityLinkCount());

        taskService.complete(taskId);
    }
}

The BPMN used is

<?xml version="1.0" encoding="UTF-8"?>
<definitions xmlns="http://www.omg.org/spec/BPMN/20100524/MODEL" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:flowable="http://flowable.org/bpmn" xmlns:bpmndi="http://www.omg.org/spec/BPMN/20100524/DI" xmlns:omgdc="http://www.omg.org/spec/DD/20100524/DC" xmlns:omgdi="http://www.omg.org/spec/DD/20100524/DI" typeLanguage="http://www.w3.org/2001/XMLSchema" expressionLanguage="http://www.w3.org/1999/XPath" targetNamespace="confix/v3">
  <process id="my-process" name="My Process" isExecutable="true">
    <startEvent id="startNode" name="Start node" flowable:initiator="initiator"></startEvent>
    <userTask id="userTask1" name="userTask1"></userTask>
    <endEvent id="endevent6" name="End"></endEvent>
    <sequenceFlow id="flow1" sourceRef="startNode" targetRef="userTask1"></sequenceFlow>
    <sequenceFlow id="flow2" sourceRef="userTask1" targetRef="endevent6"></sequenceFlow>
  </process>
</definitions>

When running the test there is the following log message and database error. In the log message we can see a negative IdentityLinkCount

12:52:26.864 [main] INFO  t.TaskIdentityLinkCountNegative - IdentityLinkCount is -1
12:52:26.919 [main] ERROR o.f.c.e.i.interceptor.CommandContext - Error while closing command context
org.apache.ibatis.exceptions.PersistenceException: 
### Error updating database.  Cause: org.h2.jdbc.JdbcSQLException: Referential integrity constraint violation: "ACT_FK_TSKASS_TASK: PUBLIC.ACT_RU_IDENTITYLINK FOREIGN KEY(TASK_ID_) REFERENCES PUBLIC.ACT_RU_TASK(ID_) ('10')"; SQL statement:
delete from ACT_RU_TASK where ID_ = ? and REV_ = ? [23503-176]
### The error may exist in org/flowable/task/service/db/mapping/entity/Task.xml
### The error may involve org.flowable.task.service.impl.persistence.entity.TaskEntityImpl.deleteTask-Inline
### The error occurred while setting parameters
### SQL: delete from ACT_RU_TASK where ID_ = ? and REV_ = ?
### Cause: org.h2.jdbc.JdbcSQLException: Referential integrity constraint violation: "ACT_FK_TSKASS_TASK: PUBLIC.ACT_RU_IDENTITYLINK FOREIGN KEY(TASK_ID_) REFERENCES PUBLIC.ACT_RU_TASK(ID_) ('10')"; SQL statement:
delete from ACT_RU_TASK where ID_ = ? and REV_ = ? [23503-176]
    at org.apache.ibatis.exceptions.ExceptionFactory.wrapException(ExceptionFactory.java:30) ~[mybatis-3.5.5.jar:3.5.5]
    at org.apache.ibatis.session.defaults.DefaultSqlSession.update(DefaultSqlSession.java:199) ~[mybatis-3.5.5.jar:3.5.5]
    at org.apache.ibatis.session.defaults.DefaultSqlSession.delete(DefaultSqlSession.java:212) ~[mybatis-3.5.5.jar:3.5.5]
    at org.flowable.common.engine.impl.db.DbSqlSession.flushDeleteEntities(DbSqlSession.java:641) ~[flowable-engine-common-6.6.0.jar:6.6.0]
    at org.flowable.common.engine.impl.db.DbSqlSession.flushDeletes(DbSqlSession.java:598) ~[flowable-engine-common-6.6.0.jar:6.6.0]
    at org.flowable.common.engine.impl.db.DbSqlSession.flush(DbSqlSession.java:365) ~[flowable-engine-common-6.6.0.jar:6.6.0]
    at org.flowable.common.engine.impl.interceptor.CommandContext.flushSessions(CommandContext.java:211) ~[flowable-engine-common-6.6.0.jar:6.6.0]
    at org.flowable.common.engine.impl.interceptor.CommandContext.close(CommandContext.java:69) ~[flowable-engine-common-6.6.0.jar:6.6.0]
    at org.flowable.common.engine.impl.interceptor.CommandContextInterceptor.execute(CommandContextInterceptor.java:92) [flowable-engine-common-6.6.0.jar:6.6.0]
    at org.flowable.common.engine.impl.interceptor.LogInterceptor.execute(LogInterceptor.java:30) [flowable-engine-common-6.6.0.jar:6.6.0]
    at org.flowable.common.engine.impl.cfg.CommandExecutorImpl.execute(CommandExecutorImpl.java:56) [flowable-engine-common-6.6.0.jar:6.6.0]
    at org.flowable.common.engine.impl.cfg.CommandExecutorImpl.execute(CommandExecutorImpl.java:51) [flowable-engine-common-6.6.0.jar:6.6.0]
    at org.flowable.engine.impl.TaskServiceImpl.complete(TaskServiceImpl.java:212) [flowable-engine-6.6.0.jar:6.6.0]
    at testcases.TaskIdentityLinkCountNegative.testUserIdentityLinkCountIsIncorrect(TaskIdentityLinkCountNegative.java:63) [test-classes/:na]
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[na:1.8.0_272]
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[na:1.8.0_272]
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[na:1.8.0_272]
    at java.lang.reflect.Method.invoke(Method.java:498) ~[na:1.8.0_272]
    at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:513) [junit-platform-commons-1.2.0.jar:1.2.0]
    at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:115) [junit-jupiter-engine-5.2.0.jar:5.2.0]
    at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeTestMethod$6(TestMethodTestDescriptor.java:170) [junit-jupiter-engine-5.2.0.jar:5.2.0]
    at org.junit.jupiter.engine.execution.ThrowableCollector.execute(ThrowableCollector.java:40) ~[junit-jupiter-engine-5.2.0.jar:5.2.0]
    at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeTestMethod(TestMethodTestDescriptor.java:166) [junit-jupiter-engine-5.2.0.jar:5.2.0]
    at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:113) [junit-jupiter-engine-5.2.0.jar:5.2.0]
    at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:58) [junit-jupiter-engine-5.2.0.jar:5.2.0]
    at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor$NodeExecutor.lambda$executeRecursively$3(HierarchicalTestExecutor.java:113) ~[junit-platform-engine-1.2.0.jar:1.2.0]
    at org.junit.platform.engine.support.hierarchical.SingleTestExecutor.executeSafely(SingleTestExecutor.java:66) ~[junit-platform-engine-1.2.0.jar:1.2.0]
    at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor$NodeExecutor.executeRecursively(HierarchicalTestExecutor.java:108) ~[junit-platform-engine-1.2.0.jar:1.2.0]
    at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor$NodeExecutor.execute(HierarchicalTestExecutor.java:79) ~[junit-platform-engine-1.2.0.jar:1.2.0]
    at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor$NodeExecutor.lambda$executeRecursively$2(HierarchicalTestExecutor.java:121) ~[junit-platform-engine-1.2.0.jar:1.2.0]
    at java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183) ~[na:1.8.0_272]
    at java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:175) ~[na:1.8.0_272]
    at java.util.Iterator.forEachRemaining(Iterator.java:116) ~[na:1.8.0_272]
    at java.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1801) ~[na:1.8.0_272]
    at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:482) ~[na:1.8.0_272]
    at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:472) ~[na:1.8.0_272]
    at java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150) ~[na:1.8.0_272]
    at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173) ~[na:1.8.0_272]
    at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) ~[na:1.8.0_272]
    at java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:485) ~[na:1.8.0_272]
    at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor$NodeExecutor.lambda$executeRecursively$3(HierarchicalTestExecutor.java:121) ~[junit-platform-engine-1.2.0.jar:1.2.0]
    at org.junit.platform.engine.support.hierarchical.SingleTestExecutor.executeSafely(SingleTestExecutor.java:66) ~[junit-platform-engine-1.2.0.jar:1.2.0]
    at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor$NodeExecutor.executeRecursively(HierarchicalTestExecutor.java:108) ~[junit-platform-engine-1.2.0.jar:1.2.0]
    at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor$NodeExecutor.execute(HierarchicalTestExecutor.java:79) ~[junit-platform-engine-1.2.0.jar:1.2.0]
    at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor$NodeExecutor.lambda$executeRecursively$2(HierarchicalTestExecutor.java:121) ~[junit-platform-engine-1.2.0.jar:1.2.0]
    at java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183) ~[na:1.8.0_272]
    at java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:175) ~[na:1.8.0_272]
    at java.util.Iterator.forEachRemaining(Iterator.java:116) ~[na:1.8.0_272]
    at java.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1801) ~[na:1.8.0_272]
    at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:482) ~[na:1.8.0_272]
    at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:472) ~[na:1.8.0_272]
    at java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150) ~[na:1.8.0_272]
    at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173) ~[na:1.8.0_272]
    at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) ~[na:1.8.0_272]
    at java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:485) ~[na:1.8.0_272]
    at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor$NodeExecutor.lambda$executeRecursively$3(HierarchicalTestExecutor.java:121) ~[junit-platform-engine-1.2.0.jar:1.2.0]
    at org.junit.platform.engine.support.hierarchical.SingleTestExecutor.executeSafely(SingleTestExecutor.java:66) ~[junit-platform-engine-1.2.0.jar:1.2.0]
    at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor$NodeExecutor.executeRecursively(HierarchicalTestExecutor.java:108) ~[junit-platform-engine-1.2.0.jar:1.2.0]
    at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor$NodeExecutor.execute(HierarchicalTestExecutor.java:79) ~[junit-platform-engine-1.2.0.jar:1.2.0]
    at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:55) ~[junit-platform-engine-1.2.0.jar:1.2.0]
    at org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:43) ~[junit-platform-engine-1.2.0.jar:1.2.0]
    at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:170) ~[junit-platform-launcher-1.2.0.jar:1.2.0]
    at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:154) ~[junit-platform-launcher-1.2.0.jar:1.2.0]
    at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:90) ~[junit-platform-launcher-1.2.0.jar:1.2.0]
    at com.intellij.junit5.JUnit5IdeaTestRunner.startRunnerWithArgs(JUnit5IdeaTestRunner.java:74) ~[junit5-rt.jar:na]
    at com.intellij.rt.execution.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:47) ~[junit-rt.jar:na]
    at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:242) ~[junit-rt.jar:na]
    at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:70) ~[junit-rt.jar:na]
Caused by: org.h2.jdbc.JdbcSQLException: Referential integrity constraint violation: "ACT_FK_TSKASS_TASK: PUBLIC.ACT_RU_IDENTITYLINK FOREIGN KEY(TASK_ID_) REFERENCES PUBLIC.ACT_RU_TASK(ID_) ('10')"; SQL statement:
delete from ACT_RU_TASK where ID_ = ? and REV_ = ? [23503-176]
    at org.h2.message.DbException.getJdbcSQLException(DbException.java:344) ~[h2-1.3.176.jar:1.3.176]
    at org.h2.message.DbException.get(DbException.java:178) ~[h2-1.3.176.jar:1.3.176]
    at org.h2.message.DbException.get(DbException.java:154) ~[h2-1.3.176.jar:1.3.176]
    at org.h2.constraint.ConstraintReferential.checkRow(ConstraintReferential.java:427) ~[h2-1.3.176.jar:1.3.176]
    at org.h2.constraint.ConstraintReferential.checkRowRefTable(ConstraintReferential.java:444) ~[h2-1.3.176.jar:1.3.176]
    at org.h2.constraint.ConstraintReferential.checkRow(ConstraintReferential.java:319) ~[h2-1.3.176.jar:1.3.176]
    at org.h2.table.Table.fireConstraints(Table.java:909) ~[h2-1.3.176.jar:1.3.176]
    at org.h2.table.Table.fireAfterRow(Table.java:927) ~[h2-1.3.176.jar:1.3.176]
    at org.h2.command.dml.Delete.update(Delete.java:101) ~[h2-1.3.176.jar:1.3.176]
    at org.h2.command.CommandContainer.update(CommandContainer.java:79) ~[h2-1.3.176.jar:1.3.176]
    at org.h2.command.Command.executeUpdate(Command.java:254) ~[h2-1.3.176.jar:1.3.176]
    at org.h2.jdbc.JdbcPreparedStatement.execute(JdbcPreparedStatement.java:199) ~[h2-1.3.176.jar:1.3.176]
    at org.apache.ibatis.executor.statement.PreparedStatementHandler.update(PreparedStatementHandler.java:47) ~[mybatis-3.5.5.jar:3.5.5]
    at org.apache.ibatis.executor.statement.RoutingStatementHandler.update(RoutingStatementHandler.java:74) ~[mybatis-3.5.5.jar:3.5.5]
    at org.apache.ibatis.executor.SimpleExecutor.doUpdate(SimpleExecutor.java:50) ~[mybatis-3.5.5.jar:3.5.5]
    at org.apache.ibatis.executor.BaseExecutor.update(BaseExecutor.java:117) ~[mybatis-3.5.5.jar:3.5.5]
    at org.apache.ibatis.executor.CachingExecutor.update(CachingExecutor.java:76) ~[mybatis-3.5.5.jar:3.5.5]
    at org.apache.ibatis.session.defaults.DefaultSqlSession.update(DefaultSqlSession.java:197) ~[mybatis-3.5.5.jar:3.5.5]
    ... 66 common frames omitted

Additional context Seen in Flowable 6.6.0 using H2 and Postgres database.

4gsim commented 3 years ago

You could add in the IdentityLinkEntityManagerImpl.deleteTaskIdentityLink a check if the entity was already deleted and not delete it again. List<IdentityLinkEntity> removedIdentityLinkEntities = new ArrayList<>(); for (IdentityLinkEntity identityLink : identityLinks) { if (!identityLink.isDeleted()) { delete(identityLink); removedIdentityLinkEntities.add(identityLink); } } But to me this seems to be a deeper problem with the caching system during the execution in the management service. There is a desynchronization between what is deleted and what is selected. The selection seems to be always from the database for lists.