Inital fix for ACT_1387

modules/activiti-engine/src/main/java/org/activiti/engine/impl/cfg/StandaloneProcessEngineConfiguration.java

@@ -28,14 +28,22 @@
 public class StandaloneProcessEngineConfiguration extends ProcessEngineConfigurationImpl {
 
   protected Collection< ? extends CommandInterceptor> getDefaultCommandInterceptorsTxRequired() {
+    return createDefaultCommandInterceptors(true);
+  }
+
+  protected Collection< ? extends CommandInterceptor> getDefaultCommandInterceptorsTxRequiresNew() {
+    return createDefaultCommandInterceptors(false);
+  }
+
+  protected Collection< ? extends CommandInterceptor> createDefaultCommandInterceptors(boolean contextReusePossible) {
     List<CommandInterceptor> defaultCommandInterceptorsTxRequired = new ArrayList<CommandInterceptor>();
     defaultCommandInterceptorsTxRequired.add(new LogInterceptor());
-    defaultCommandInterceptorsTxRequired.add(new CommandContextInterceptor(commandContextFactory, this));
+
+    CommandContextInterceptor commandContextInterceptor = new CommandContextInterceptor(commandContextFactory, this);
+    commandContextInterceptor.setContextReusePossible(contextReusePossible);
+    defaultCommandInterceptorsTxRequired.add(commandContextInterceptor);
+
     return defaultCommandInterceptorsTxRequired;
   }
 
-  protected Collection< ? extends CommandInterceptor> getDefaultCommandInterceptorsTxRequiresNew() {
-    // assumes this is already initialized and in standalone cases the required and requires new are the same
-    return commandInterceptorsTxRequired;
-  }
 }

modules/activiti-engine/src/main/java/org/activiti/engine/impl/db/DbIdGenerator.java

@@ -38,7 +38,6 @@ public synchronized String getNextId() {
   }
 
   protected synchronized void getNewBlock() {
-    // TODO http://jira.codehaus.org/browse/ACT-45 use a separate 'requiresNew' command executor
     IdBlock idBlock = commandExecutor.execute(new GetNextIdBlockCmd(idBlockSize));
     this.nextId = idBlock.getNextId();
     this.lastId = idBlock.getLastId();

modules/activiti-engine/src/main/java/org/activiti/engine/impl/interceptor/CommandContextInterceptor.java

@@ -14,16 +14,21 @@
 package org.activiti.engine.impl.interceptor;
 
 
+import java.util.logging.Logger;
+
 import org.activiti.engine.impl.cfg.ProcessEngineConfigurationImpl;
 import org.activiti.engine.impl.context.Context;
 
 /**
  * @author Tom Baeyens
  */
 public class CommandContextInterceptor extends CommandInterceptor {
+
+  private final Logger log = Logger.getLogger(CommandContextInterceptor.class.getName());
 
   protected CommandContextFactory commandContextFactory;
   protected ProcessEngineConfigurationImpl processEngineConfiguration;
+  protected boolean isContextReusePossible; // For backwards compatibility, the default is 'false'
 
   public CommandContextInterceptor() {
   }
@@ -34,28 +39,68 @@ public CommandContextInterceptor(CommandContextFactory commandContextFactory, Pr
   }
 
   public <T> T execute(Command<T> command) {
-    CommandContext context = commandContextFactory.createCommandContext(command);
+    CommandContext context = Context.getCommandContext();
+
+    boolean contextReused = false;
+    // We need to check the exception, because the transaction can be in a rollback state,
+    // and some other command is being fired to compensate (eg. decrementing job retries)
+    if (!isContextReusePossible || context == null || context.getException() != null) { 
+    	context = commandContextFactory.createCommandContext(command);    	
+    }  
+    else {
+    	log.fine("Valid context found. Reusing it for the current comment '" + command.getClass().getCanonicalName() + "'");
+    	contextReused = true;
+    }
 
     try {
+      // Push on stack
       Context.setCommandContext(context);
       Context.setProcessEngineConfiguration(processEngineConfiguration);
+
       return next.execute(command);
 
     } catch (Exception e) {
+
       context.exception(e);
 
     } finally {
       try {
-        context.close();
+    	  if (!contextReused) {
+    		  context.close();
+    	  }
       } finally {
-        Context.removeCommandContext();
-        Context.removeProcessEngineConfiguration();
+    	  // Pop from stack
+    	  Context.removeCommandContext();
+    	  Context.removeProcessEngineConfiguration();
       }
     }
 
     return null;
   }
 
+//  public <T> T execute(Command<T> command) {
+//    CommandContext context = commandContextFactory.createCommandContext(command);
+//
+//    try {
+//      Context.setCommandContext(context);
+//      Context.setProcessEngineConfiguration(processEngineConfiguration);
+//      return next.execute(command);
+//      
+//    } catch (Exception e) {
+//      context.exception(e);
+//      
+//    } finally {
+//      try {
+//        context.close();
+//      } finally {
+//        Context.removeCommandContext();
+//        Context.removeProcessEngineConfiguration();
+//      }
+//    }
+//    
+//    return null;
+//  }
+
   public CommandContextFactory getCommandContextFactory() {
     return commandContextFactory;
   }
@@ -71,4 +116,15 @@ public ProcessEngineConfigurationImpl getProcessEngineConfiguration() {
   public void setProcessEngineContext(ProcessEngineConfigurationImpl processEngineContext) {
     this.processEngineConfiguration = processEngineContext;
   }
+
+
+  public boolean isContextReusePossible() {
+    return isContextReusePossible;
+  }
+
+
+  public void setContextReusePossible(boolean isContextReusePossible) {
+    this.isContextReusePossible = isContextReusePossible;
+  }
+
 }

modules/activiti-engine/src/test/java/org/activiti/engine/test/bpmn/servicetask/CallServiceInServiceTaskTest.java

@@ -0,0 +1,66 @@
+/* Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * 
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.activiti.engine.test.bpmn.servicetask;
+
+import java.util.List;
+
+import org.activiti.engine.impl.test.PluggableActivitiTestCase;
+import org.activiti.engine.repository.ProcessDefinition;
+import org.activiti.engine.runtime.ProcessInstance;
+import org.activiti.engine.test.Deployment;
+
+/**
+ * @author Joram Barrez
+ */
+public class CallServiceInServiceTaskTest extends PluggableActivitiTestCase {
+
+  @Deployment
+	public void testStartProcessFromDelegate() {
+    runtimeService.startProcessInstanceByKey("startProcessFromDelegate");
+
+    // Starting the process should lead to two processes being started,
+    // The other one started from the java delegate in the service task
+    List<ProcessInstance> processInstances = runtimeService.createProcessInstanceQuery().list();
+    assertEquals(2, processInstances.size());
+
+    boolean startProcessFromDelegateFound = false;
+    boolean oneTaskProcessFound = false;
+    for (ProcessInstance processInstance : processInstances) {
+      ProcessDefinition processDefinition = repositoryService.getProcessDefinition(processInstance.getProcessDefinitionId());
+      if (processDefinition.getKey().equals("startProcessFromDelegate")) {
+        startProcessFromDelegateFound = true;
+      } else if (processDefinition.getKey().equals("oneTaskProcess")) {
+        oneTaskProcessFound = true;
+      }
+    }
+
+    assertTrue(startProcessFromDelegateFound);
+    assertTrue(oneTaskProcessFound);
+  }
+
+  @Deployment
+  public void testRollBackOnException() {
+    Exception expectedException = null;
+    try {
+      runtimeService.startProcessInstanceByKey("startProcessFromDelegate");
+    } catch (Exception e) {
+      expectedException = e;
+    }
+    assertNotNull(expectedException);
+
+    // Starting the process should cause a rollback of both processes
+    assertEquals(0, runtimeService.createProcessInstanceQuery().count());
+  }
+
+
+}

modules/activiti-engine/src/test/java/org/activiti/engine/test/bpmn/servicetask/StartProcessInstanceTestDelegate.java

@@ -0,0 +1,30 @@
+/* Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * 
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.activiti.engine.test.bpmn.servicetask;
+
+import org.activiti.engine.RuntimeService;
+import org.activiti.engine.delegate.DelegateExecution;
+import org.activiti.engine.delegate.JavaDelegate;
+import org.activiti.engine.impl.context.Context;
+
+/**
+ * @author Joram Barrez
+ */
+public class StartProcessInstanceTestDelegate implements JavaDelegate {
+
+  public void execute(DelegateExecution execution) throws Exception {
+    RuntimeService runtimeService = Context.getProcessEngineConfiguration().getRuntimeService();
+    runtimeService.startProcessInstanceByKey("oneTaskProcess");
+  }
+
+}

modules/activiti-engine/src/test/resources/org/activiti/engine/test/bpmn/servicetask/CallServiceInServiceTaskTest.testRollBackOnException.bpmn20.xml

@@ -0,0 +1,39 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<definitions 
+  xmlns="http://www.omg.org/spec/BPMN/20100524/MODEL"
+  xmlns:activiti="http://activiti.org/bpmn"
+  targetNamespace="Examples">
+
+  <process id="startProcessFromDelegate" isExecutable="true">
+
+    <startEvent id="theStart" />
+    <sequenceFlow id="flow1" sourceRef="theStart" targetRef="service" />
+
+    <serviceTask id="service" activiti:class="org.activiti.engine.test.bpmn.servicetask.StartProcessInstanceTestDelegate" />
+    <sequenceFlow id="flow2" sourceRef="service" targetRef="failingScriptTask" />
+
+    <scriptTask id="failingScriptTask" name="Execute script" scriptFormat="groovy">
+        <script>
+            // Throw an exception in script task
+            throw new RuntimeException("This is an exception thrown from scriptTask")
+        </script>
+    </scriptTask>
+    <sequenceFlow id="flow3" sourceRef="failingScriptTask" targetRef="theEnd" />
+
+    <endEvent id="theEnd" />
+
+  </process>
+
+  <process id="oneTaskProcess" isExecutable="true">
+
+    <startEvent id="oneTaskProcessStart" />
+    <sequenceFlow id="oneTaskProcessFlow1" sourceRef="oneTaskProcessStart" targetRef="oneTaskProcessUserTask" />
+
+    <userTask id="oneTaskProcessUserTask" />
+    <sequenceFlow id="oneTaskProcessFlow2" sourceRef="oneTaskProcessUserTask" targetRef="oneTaskProcessEnd" />
+
+    <endEvent id="oneTaskProcessEnd" />
+
+  </process>
+
+</definitions>

modules/activiti-engine/src/test/resources/org/activiti/engine/test/bpmn/servicetask/CallServiceInServiceTaskTest.testStartProcessFromDelegate.bpmn20.xml

@@ -0,0 +1,34 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<definitions 
+  xmlns="http://www.omg.org/spec/BPMN/20100524/MODEL"
+  xmlns:activiti="http://activiti.org/bpmn"
+  targetNamespace="Examples">
+
+  <process id="startProcessFromDelegate" isExecutable="true">
+
+    <startEvent id="theStart" />
+    <sequenceFlow id="flow1" sourceRef="theStart" targetRef="service" />
+
+    <serviceTask id="service" activiti:class="org.activiti.engine.test.bpmn.servicetask.StartProcessInstanceTestDelegate" />
+    <sequenceFlow id="flow2" sourceRef="service" targetRef="task" />
+
+    <userTask id="task" />
+    <sequenceFlow id="flow3" sourceRef="task" targetRef="theEnd" />
+
+    <endEvent id="theEnd" />
+
+  </process>
+
+  <process id="oneTaskProcess" isExecutable="true">
+
+    <startEvent id="oneTaskProcessStart" />
+    <sequenceFlow id="oneTaskProcessFlow1" sourceRef="oneTaskProcessStart" targetRef="oneTaskProcessTask" />
+
+    <userTask id="oneTaskProcessTask" />
+    <sequenceFlow id="oneTaskProcessFlow2" sourceRef="oneTaskProcessTask" targetRef="oneTaskProcessEnd" />
+
+    <endEvent id="oneTaskProcessEnd" />
+
+  </process>
+
+</definitions>

modules/activiti-spring/src/main/java/org/activiti/spring/SpringProcessEngineConfiguration.java

@@ -71,6 +71,7 @@ public ProcessEngine buildProcessEngine() {
     defaultCommandInterceptorsTxRequired.add(new LogInterceptor());
     defaultCommandInterceptorsTxRequired.add(new SpringTransactionInterceptor(transactionManager, TransactionTemplate.PROPAGATION_REQUIRED));
     CommandContextInterceptor commandContextInterceptor = new CommandContextInterceptor(commandContextFactory, this);
+    commandContextInterceptor.setContextReusePossible(true);
     defaultCommandInterceptorsTxRequired.add(commandContextInterceptor);
     return defaultCommandInterceptorsTxRequired;
   }
@@ -80,6 +81,7 @@ public ProcessEngine buildProcessEngine() {
     defaultCommandInterceptorsTxRequiresNew.add(new LogInterceptor());
     defaultCommandInterceptorsTxRequiresNew.add(new SpringTransactionInterceptor(transactionManager, TransactionTemplate.PROPAGATION_REQUIRES_NEW));
     CommandContextInterceptor commandContextInterceptor = new CommandContextInterceptor(commandContextFactory, this);
+    commandContextInterceptor.setContextReusePossible(false);
     defaultCommandInterceptorsTxRequiresNew.add(commandContextInterceptor);
     return defaultCommandInterceptorsTxRequiresNew;
   }