Merge branch 'master' of https://github.com/Activiti/Activiti

modules/activiti-engine/src/main/java/org/activiti/engine/TaskService.java

@@ -54,16 +54,21 @@ public interface TaskService {
 	void saveTask(Task task);
 	
 	/**
-	 * Deletes the given task.
+	 * Deletes the given task, not deleting historic information that is related to this task.
 	 * @param taskId The id of the task that will be deleted, cannot be null. If no task
 	 * exists with the given taskId, the operation is ignored.
+	 * @throws ActivitiException when an error occurs while deleting the task or in case the task is part
+   *   of a running process.
 	 */
 	void deleteTask(String taskId);
 	
 	/**
-	 * Deletes all tasks of the given collection.
+	 * Deletes all tasks of the given collection, not deleting historic information that is related 
+	 * to these tasks.
 	 * @param taskIds The id's of the tasks that will be deleted, cannot be null. All
 	 * id's in the list that don't have an existing task will be ignored.
+	 * @throws ActivitiException when an error occurs while deleting the tasks or in case one of the tasks
+   *  is part of a running process.
 	 */
 	void deleteTasks(Collection<String> taskIds);
 	
@@ -72,6 +77,8 @@ public interface TaskService {
    * @param taskId The id of the task that will be deleted, cannot be null. If no task
    * exists with the given taskId, the operation is ignored.
    * @param cascade If cascade is true, also the historic information related to this task is deleted.
+   * @throws ActivitiException when an error occurs while deleting the task or in case the task is part
+   *   of a running process.
    */
   void deleteTask(String taskId, boolean cascade);
   
@@ -80,9 +87,33 @@ public interface TaskService {
    * @param taskIds The id's of the tasks that will be deleted, cannot be null. All
    * id's in the list that don't have an existing task will be ignored.
    * @param cascade If cascade is true, also the historic information related to this task is deleted.
+   * @throws ActivitiException when an error occurs while deleting the tasks or in case one of the tasks
+   *  is part of a running process.
+   * @throws ActivitiException when an error occurs while deleting the tasks or in case one of the tasks
+   *  is part of a running process.
    */
   void deleteTasks(Collection<String> taskIds, boolean cascade);
   
+  /**
+   * Deletes the given task, not deleting historic information that is related to this task..
+   * @param taskId The id of the task that will be deleted, cannot be null. If no task
+   * exists with the given taskId, the operation is ignored.
+   * @param deleteReason reason the task is deleted. Is recorded in history, if enabled.
+   * @throws ActivitiException when an error occurs while deleting the task or in case the task is part
+   *  of a running process
+   */
+  void deleteTask(String taskId, String deleteReason);
+  
+  /**
+   * Deletes all tasks of the given collection, not deleting historic information that is related to these tasks.
+   * @param taskIds The id's of the tasks that will be deleted, cannot be null. All
+   * id's in the list that don't have an existing task will be ignored.
+   * @param deleteReason reason the task is deleted. Is recorded in history, if enabled.
+   * @throws ActivitiException when an error occurs while deleting the tasks or in case one of the tasks
+   *  is part of a running process.
+   */
+  void deleteTasks(Collection<String> taskIds, String deleteReason);
+  
 	 /**
    * Claim responsibility for a task: the given user is made assignee for the task.
    * The difference with {@link #setAssignee(String, String)} is that here 

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

@@ -79,19 +79,29 @@ public class TaskServiceImpl extends ServiceImpl implements TaskService {
   }
   
   public void deleteTask(String taskId) {
-    commandExecutor.execute(new DeleteTaskCmd(taskId, false));
+    commandExecutor.execute(new DeleteTaskCmd(taskId, null, false));
   }
   
   public void deleteTasks(Collection<String> taskIds) {
-    commandExecutor.execute(new DeleteTaskCmd(taskIds, false));
+    commandExecutor.execute(new DeleteTaskCmd(taskIds, null, false));
   }
   
   public void deleteTask(String taskId, boolean cascade) {
-    commandExecutor.execute(new DeleteTaskCmd(taskId, cascade));
+    commandExecutor.execute(new DeleteTaskCmd(taskId, null, cascade));
   }
 
   public void deleteTasks(Collection<String> taskIds, boolean cascade) {
-    commandExecutor.execute(new DeleteTaskCmd(taskIds, cascade));
+    commandExecutor.execute(new DeleteTaskCmd(taskIds, null, cascade));
+  }
+  
+  @Override
+  public void deleteTask(String taskId, String deleteReason) {
+    commandExecutor.execute(new DeleteTaskCmd(taskId, deleteReason, false));
+  }
+  
+  @Override
+  public void deleteTasks(Collection<String> taskIds, String deleteReason) {
+    commandExecutor.execute(new DeleteTaskCmd(taskIds, deleteReason, false));
   }
 
   public void setAssignee(String taskId, String userId) {

modules/activiti-engine/src/main/java/org/activiti/engine/impl/cmd/DeleteTaskCmd.java

@@ -30,15 +30,18 @@ public class DeleteTaskCmd implements Command<Void>, Serializable {
   protected String taskId;
   protected Collection<String> taskIds;
   protected boolean cascade;
+  protected String deleteReason;
   
-  public DeleteTaskCmd(String taskId, boolean cascade) {
+  public DeleteTaskCmd(String taskId, String deleteReason, boolean cascade) {
     this.taskId = taskId;
     this.cascade = cascade;
+    this.deleteReason = deleteReason;
   }
   
-  public DeleteTaskCmd(Collection<String> taskIds, boolean cascade) {
+  public DeleteTaskCmd(Collection<String> taskIds, String deleteReason, boolean cascade) {
     this.taskIds = taskIds;
     this.cascade = cascade;
+    this.deleteReason = deleteReason;
   }
 
   public Void execute(CommandContext commandContext) {
@@ -60,6 +63,6 @@ public class DeleteTaskCmd implements Command<Void>, Serializable {
     Context
       .getCommandContext()
       .getTaskManager()
-      .deleteTask(taskId, cascade);
+      .deleteTask(taskId, deleteReason, cascade);
   }
 }

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

@@ -91,11 +91,11 @@ public class TaskManager extends AbstractManager {
     return getDbSqlSession().selectList("selectTasksByExecutionId", executionId);
   }
   
+  @SuppressWarnings("unchecked")
   public List<TaskEntity> findTasksByProcessInstanceId(String processInstanceId) {
     return getDbSqlSession().selectList("selectTasksByProcessInstanceId", processInstanceId);
   }
   
-  @SuppressWarnings("unchecked")
   @Deprecated
   public List<Task> findTasksByQueryCriteria(TaskQueryImpl taskQuery, Page page) {
     taskQuery.setFirstResult(page.getFirstResult());
@@ -127,15 +127,19 @@ public class TaskManager extends AbstractManager {
     return getDbSqlSession().selectList("selectTasksByParentTaskId", parentTaskId);
   }
 
-  public void deleteTask(String taskId, boolean cascade) {
+  public void deleteTask(String taskId, String deleteReason, boolean cascade) {
     TaskEntity task = Context
       .getCommandContext()
       .getTaskManager()
       .findTaskById(taskId);
     
     if (task!=null) {
-      deleteTask(task, TaskEntity.DELETE_REASON_DELETED, cascade);
-
+      if(task.getExecutionId() != null) {
+        throw new ActivitiException("The task cannot be deleted because is part of a running process");
+      }
+      
+      String reason = (deleteReason == null || deleteReason.length() == 0) ? TaskEntity.DELETE_REASON_DELETED : deleteReason;
+      deleteTask(task, reason, cascade);
     } else if (cascade) {
       Context
         .getCommandContext()

modules/activiti-engine/src/test/java/org/activiti/engine/test/api/task/TaskServiceTest.java

@@ -495,8 +495,6 @@ public class TaskServiceTest extends PluggableActivitiTestCase {
     Map<String, Object> variables = runtimeService.getVariables(processInstance.getId());
     assertEquals(1, variables.size());
     assertEquals("myValue", variables.get("myParam"));
-
-    taskService.deleteTask(task.getId(), true);
   }
   
   public void testSetAssignee() {
@@ -1055,4 +1053,72 @@ public class TaskServiceTest extends PluggableActivitiTestCase {
     }
   }
   
+  public void testDeleteTaskWithDeleteReason() {
+    // ACT-900: deleteReason can be manually specified - can only be validated when historyLevel > ACTIVITY
+    if (processEngineConfiguration.getHistoryLevel().isAtLeast(HistoryLevel.ACTIVITY)) {
+      
+      Task task = taskService.newTask();
+      task.setName("test task");
+      taskService.saveTask(task);
+      
+      assertNotNull(task.getId());
+      
+      taskService.deleteTask(task.getId(), "deleted for testing purposes");
+      
+      HistoricTaskInstance historicTaskInstance = historyService.createHistoricTaskInstanceQuery()
+        .taskId(task.getId()).singleResult();
+      
+      assertNotNull(historicTaskInstance);
+      assertEquals("deleted for testing purposes", historicTaskInstance.getDeleteReason());
+      
+      // Delete historic task that is left behind, will not be cleaned up because this is not part of a process
+      taskService.deleteTask(task.getId(), true);
+      
+    }
+  }
+  
+  @Deployment(resources = { "org/activiti/engine/test/api/oneTaskProcess.bpmn20.xml" })
+  public void testDeleteTaskPartOfProcess() {
+    runtimeService.startProcessInstanceByKey("oneTaskProcess");
+    Task task = taskService.createTaskQuery().singleResult();
+    assertNotNull(task);
+    
+    try {
+      taskService.deleteTask(task.getId());
+    } catch(ActivitiException ae) {
+      assertEquals("The task cannot be deleted because is part of a running process", ae.getMessage());
+    }
+    
+    try {
+      taskService.deleteTask(task.getId(), true);
+    } catch(ActivitiException ae) {
+      assertEquals("The task cannot be deleted because is part of a running process", ae.getMessage());
+    }
+    
+    try {
+      taskService.deleteTask(task.getId(), "test");
+    } catch(ActivitiException ae) {
+      assertEquals("The task cannot be deleted because is part of a running process", ae.getMessage());
+    }
+    
+    try {
+      taskService.deleteTasks(Arrays.asList(task.getId()));
+    } catch(ActivitiException ae) {
+      assertEquals("The task cannot be deleted because is part of a running process", ae.getMessage());
+    }
+    
+    try {
+      taskService.deleteTasks(Arrays.asList(task.getId()), true);
+    } catch(ActivitiException ae) {
+      assertEquals("The task cannot be deleted because is part of a running process", ae.getMessage());
+    }
+    
+    try {
+      taskService.deleteTasks(Arrays.asList(task.getId()), "test");
+    } catch(ActivitiException ae) {
+      assertEquals("The task cannot be deleted because is part of a running process", ae.getMessage());
+    }
+    
+  }
+  
 }

modules/activiti-engine/src/test/java/org/activiti/engine/test/db/ProcessInstanceSuspensionTest.java

 package org.activiti.engine.test.db;
 
+import java.util.Calendar;
 import java.util.Date;
+import java.util.List;
 
+import org.activiti.engine.impl.Page;
 import org.activiti.engine.impl.cmd.AcquireJobsCmd;
 import org.activiti.engine.impl.interceptor.Command;
 import org.activiti.engine.impl.interceptor.CommandContext;
+import org.activiti.engine.impl.interceptor.CommandExecutor;
 import org.activiti.engine.impl.jobexecutor.AcquiredJobs;
+import org.activiti.engine.impl.jobexecutor.GetUnlockedTimersByDuedateCmd;
+import org.activiti.engine.impl.persistence.entity.TimerEntity;
 import org.activiti.engine.impl.test.PluggableActivitiTestCase;
 import org.activiti.engine.impl.util.ClockUtil;
 import org.activiti.engine.repository.ProcessDefinition;
@@ -68,6 +74,42 @@ public class ProcessInstanceSuspensionTest extends PluggableActivitiTestCase {
     acquiredJobs = executeAcquireJobsCommand();
     assertEquals(0, acquiredJobs.size());
   }
+  
+  @Deployment
+  public void testSuspendedProcessTimerExecution() throws Exception {
+    // Process with boundary timer-event that fires in 1 hour
+    ProcessInstance procInst = runtimeService.startProcessInstanceByKey("suspendProcess");
+    assertNotNull(procInst);
+    assertEquals(1, managementService.createJobQuery().processInstanceId(procInst.getId()).count());
+    
+    // Shutdown the job-executor so timer's won't be executed
+    boolean wasJobExecutorActive = processEngineConfiguration.getJobExecutor().isActive();
+    if(wasJobExecutorActive) {
+      processEngineConfiguration.getJobExecutor().shutdown();
+    }
+    
+    // Roll time ahead to be sure timer is due to fire
+    Calendar tomorrow = Calendar.getInstance();
+    tomorrow.add(Calendar.DAY_OF_YEAR, 1);
+    ClockUtil.setCurrentTime(tomorrow.getTime());
+    
+    // Check if timer is eligable to be executed, when process in not yet suspended
+    CommandExecutor commandExecutor = processEngineConfiguration.getCommandExecutorTxRequired();
+    List<TimerEntity> jobs = commandExecutor.execute(new GetUnlockedTimersByDuedateCmd(ClockUtil.getCurrentTime(), new Page(0, 1)));
+    assertEquals(1, jobs.size());
+    
+    // Suspend process instancd
+    runtimeService.suspendProcessInstanceById(procInst.getId());
+
+    // Check if the timer is NOT aquired, even though the duedate is reached
+    jobs = commandExecutor.execute(new GetUnlockedTimersByDuedateCmd(ClockUtil.getCurrentTime(), new Page(0, 1)));
+    assertEquals(0, jobs.size());
+    
+    // Start job-executor again, if needed
+    if(wasJobExecutorActive) {
+      processEngineConfiguration.getJobExecutor().start();
+    }
+  }
 
   protected void makeSureJobDue(final Job job) {
     processEngineConfiguration.getCommandExecutorTxRequired()

modules/activiti-engine/src/test/java/org/activiti/engine/test/jobexecutor/JobExecutorTest.java

@@ -67,35 +67,4 @@ public class JobExecutorTest extends JobExecutorTestCase {
     
     assertEquals(new TreeSet<String>(expectedMessages), new TreeSet<String>(messages));
   }
-  
-  @Deployment
-  public void testSuspendedProcessTimerExecution() throws Exception {
-    // Process with boundary timer-event that fires in 1 hour
-    ProcessInstance procInst = runtimeService.startProcessInstanceByKey("suspendProcess");
-    assertNotNull(procInst);
-    assertEquals(1, managementService.createJobQuery().processInstanceId(procInst.getId()).count());
-    
-    // Shutdown the job-executor so timer's won't be executed
-    processEngineConfiguration.getJobExecutor().shutdown();
-    
-    // Roll time ahead to be sure timer is due to fire
-    Calendar tomorrow = Calendar.getInstance();
-    tomorrow.add(Calendar.DAY_OF_YEAR, 1);
-    ClockUtil.setCurrentTime(tomorrow.getTime());
-    
-    // Check if timer is eligable to be executed, when process in not yet suspended
-    CommandExecutor commandExecutor = processEngineConfiguration.getCommandExecutorTxRequired();
-    List<TimerEntity> jobs = commandExecutor.execute(new GetUnlockedTimersByDuedateCmd(ClockUtil.getCurrentTime(), new Page(0, 1)));
-    assertEquals(1, jobs.size());
-    
-    // Suspend process instancd
-    runtimeService.suspendProcessInstanceById(procInst.getId());
-
-    // Check if the timer is NOT aquired, even though the duedate is reached
-    jobs = commandExecutor.execute(new GetUnlockedTimersByDuedateCmd(ClockUtil.getCurrentTime(), new Page(0, 1)));
-    assertEquals(0, jobs.size());
-    
-    // Start job-executor again
-    processEngineConfiguration.getJobExecutor().start();
-  }
 }