ACT-1300/HEMERA-2522: moved writing of HistoricProcessVariable to process end...

ACT-1300/HEMERA-2522: moved writing of HistoricProcessVariable to process end event (not on BPMN end event), fixed name of variableValueEquals

modules/activiti-engine/src/main/java/org/activiti/engine/history/HistoricProcessVariableQuery.java

@@ -35,7 +35,7 @@ public interface HistoricProcessVariableQuery extends Query<HistoricProcessVaria
   /**
    * only select historic process variables with the given name and value
    */
-  HistoricProcessVariableQuery variableEquals(String variableName, Object variableValue);
+  HistoricProcessVariableQuery variableValueEquals(String variableName, Object variableValue);
 
   HistoricProcessVariableQuery orderByProcessInstanceId();
   

modules/activiti-engine/src/main/java/org/activiti/engine/impl/HistoricProcessVariableQueryImpl.java

@@ -78,7 +78,7 @@ public class HistoricProcessVariableQueryImpl extends AbstractQuery<HistoricProc
     return this;
   }
 
-  public HistoricProcessVariableQuery variableEquals(String variableName, Object variableValue) {
+  public HistoricProcessVariableQuery variableValueEquals(String variableName, Object variableValue) {
     if (variableName == null) {
       throw new ActivitiException("variableName is null");
     }

modules/activiti-engine/src/main/java/org/activiti/engine/impl/history/handler/HistoryParseListener.java

@@ -60,6 +60,9 @@ public class HistoryParseListener implements BpmnParseListener {
     if (activityHistoryEnabled(processDefinition, historyLevel)) {
       processDefinition.addExecutionListener(org.activiti.engine.impl.pvm.PvmEvent.EVENTNAME_END, new ProcessInstanceEndHandler());
     }
+    if (variableHistoryEnabled(processDefinition, historyLevel)) {
+      processDefinition.addExecutionListener(org.activiti.engine.impl.pvm.PvmEvent.EVENTNAME_END, HISTORIC_PROCESS_VARIABLE_HANDLER);
+    }
   }
 
   public void parseExclusiveGateway(Element exclusiveGwElement, ScopeImpl scope, ActivityImpl activity) {
@@ -124,9 +127,6 @@ public class HistoryParseListener implements BpmnParseListener {
 
   public void parseEndEvent(Element endEventElement, ScopeImpl scope, ActivityImpl activity) {
     addActivityHandlers(activity);
-    if (variableHistoryEnabled(activity,historyLevel)) {
-      activity.addExecutionListener(org.activiti.engine.impl.pvm.PvmEvent.EVENTNAME_END, HISTORIC_PROCESS_VARIABLE_HANDLER, -1);
-    }
   }
 
   public void parseParallelGateway(Element parallelGwElement, ScopeImpl scope, ActivityImpl activity) {

modules/activiti-engine/src/main/java/org/activiti/engine/impl/persistence/entity/ExecutionManager.java

@@ -64,14 +64,15 @@ public class ExecutionManager extends AbstractManager {
     commandContext
       .getTaskManager()
       .deleteTasksByProcessInstanceId(processInstanceId, deleteReason, cascade);
-
+    
+    // delete the execution BEFORE we delete the history, otherwise we will produce orphan HistoricProcessVariable instances
+    execution.deleteCascade(deleteReason);
+    
     if (cascade) {
       commandContext
       .getHistoricProcessInstanceManager()
       .deleteHistoricProcessInstanceById(processInstanceId);
     }
-    
-    execution.deleteCascade(deleteReason);
   }
 
   public ExecutionEntity findSubProcessInstanceBySuperExecutionId(String superExecutionId) {

modules/activiti-engine/src/main/java/org/activiti/engine/impl/persistence/entity/HistoricProcessVariableManager.java

@@ -47,18 +47,27 @@ public class HistoricProcessVariableManager extends AbstractHistoricManager {
   @SuppressWarnings("unchecked")
   public void deleteHistoricProcessVariableByProcessInstanceId(String historicProcessInstanceId) {
     if (historyLevel >= ProcessEngineConfigurationImpl.HISTORYLEVEL_VARIABLE) {
+      HistoricProcessVariableManager historicProcessVariableManager = Context
+              .getCommandContext()
+              .getHistoricProcessVariableManager();
+
+      // delete entries in DB
       List<HistoricProcessVariableEntity> historicProcessVariables = (List) getDbSqlSession()
         .createHistoricProcessVariableQuery()
         .processInstanceId(historicProcessInstanceId)
         .list();
-      
-      HistoricProcessVariableManager historicProcessVariableManager = Context
-        .getCommandContext()
-        .getHistoricProcessVariableManager();
-      
       for (HistoricProcessVariableEntity historicProcessVariable : historicProcessVariables) {
         historicProcessVariableManager.deleteHistoricProcessVariable(historicProcessVariable);
       }
+      
+      //delete enrties in Cache
+      List<HistoricProcessVariableEntity> cachedHistoricProcessVariables = getDbSqlSession().findInCache(HistoricProcessVariableEntity.class);
+      for (HistoricProcessVariableEntity historicProcessVariable : cachedHistoricProcessVariables) {
+        // make sure we only delete the right ones (as we cannot make a proper query in the cache)
+        if (historicProcessVariable.getProcessInstanceId().equals(historicProcessInstanceId )) {
+          historicProcessVariableManager.deleteHistoricProcessVariable(historicProcessVariable);
+        }
+      }
     }
   }
   

modules/activiti-engine/src/test/java/org/activiti/engine/test/history/HistoricProcessVariableTest.java

@@ -13,6 +13,7 @@
 
 package org.activiti.engine.test.history;
 
+import java.util.HashMap;
 import java.util.List;
 
 import org.activiti.engine.ProcessEngineConfiguration;
@@ -215,10 +216,7 @@ public class HistoricProcessVariableTest extends AbstractActivitiTestCase {
     assertEquals(4, historyService.createHistoricProcessVariableQuery().orderByTime().asc().count());
     assertEquals(4, historyService.createHistoricProcessVariableQuery().orderByVariableName().asc().count());
     
-    assertEquals(4, historyService.createHistoricProcessVariableQuery().excludeTaskDetails().count());
     assertEquals(2, historyService.createHistoricProcessVariableQuery().processInstanceId(processInstance.getId()).count());
-    assertEquals(4, historyService.createHistoricProcessVariableQuery().activityInstanceId(null).count());
-    assertEquals(0, historyService.createHistoricProcessVariableQuery().taskId("").count());
     assertEquals(2, historyService.createHistoricProcessVariableQuery().variableName("myVar").count());
     assertEquals(2, historyService.createHistoricProcessVariableQuery().variableNameLike("myVar1").count());
     
@@ -245,4 +243,18 @@ public class HistoricProcessVariableTest extends AbstractActivitiTestCase {
     assertEquals(7, historyService.createHistoricActivityInstanceQuery().count());
     assertEquals(5, historyService.createHistoricDetailQuery().count());
   }
+  
+  @Deployment(resources={
+          "org/activiti/engine/test/api/oneTaskProcess.bpmn20.xml"
+  })
+  public void testHidtoricProcessVariableOnDeletion() {
+    HashMap<String, Object> variables = new HashMap<String,  Object>();
+    variables.put("testVar", "Hallo Christian");
+    ProcessInstance processInstance = runtimeService.startProcessInstanceByKey("oneTaskProcess", variables);
+    runtimeService.deleteProcessInstance(processInstance.getId(), "deleted");
+    assertProcessEnded(processInstance.getId());
+    
+    // check that process variable is set even if the process is canceled and not ended normally
+    assertEquals(1, historyService.createHistoricProcessVariableQuery().processInstanceId(processInstance.getId()).variableValueEquals("testVar", "Hallo Christian").count());
+  }
 }