BATCH-1668: added check for transaction in JobRepository

spring-batch-core/src/main/java/org/springframework/batch/core/repository/support/AbstractJobRepositoryFactoryBean.java

@@ -16,6 +16,8 @@
 
 package org.springframework.batch.core.repository.support;
 
+import org.aopalliance.intercept.MethodInterceptor;
+import org.aopalliance.intercept.MethodInvocation;
 import org.springframework.aop.framework.ProxyFactory;
 import org.springframework.aop.support.DefaultPointcutAdvisor;
 import org.springframework.aop.support.NameMatchMethodPointcut;
@@ -29,6 +31,7 @@ import org.springframework.beans.factory.FactoryBean;
 import org.springframework.beans.factory.InitializingBean;
 import org.springframework.transaction.PlatformTransactionManager;
 import org.springframework.transaction.interceptor.TransactionInterceptor;
+import org.springframework.transaction.support.TransactionSynchronizationManager;
 import org.springframework.util.Assert;
 
 /**
@@ -51,6 +54,8 @@ public abstract class AbstractJobRepositoryFactoryBean implements FactoryBean, I
 
 	private String isolationLevelForCreate = DEFAULT_ISOLATION_LEVEL;
 
+	private boolean validateTransactionState = true;
+
 	/**
 	 * Default value for isolation level in create* method.
 	 */
@@ -90,6 +95,18 @@ public abstract class AbstractJobRepositoryFactoryBean implements FactoryBean, I
 		return true;
 	}
 
+	/**
+	 * Flag to determine whether to check for an existing transaction when a
+	 * JobExecution is created. Defaults to true because it is usually a
+	 * mistake, and leads to problems with restartability and also to deadlocks
+	 * in multi-threaded steps.
+	 * 
+	 * @param validateTransactionState the flag to set
+	 */
+	public void setValidateTransactionState(boolean validateTransactionState) {
+		this.validateTransactionState = validateTransactionState;
+	}
+
 	/**
 	 * public setter for the isolation level to be used for the transaction when
 	 * job execution entities are initially created. The default is
@@ -136,15 +153,27 @@ public abstract class AbstractJobRepositoryFactoryBean implements FactoryBean, I
 	private void initializeProxy() throws Exception {
 		if (proxyFactory == null) {
 			proxyFactory = new ProxyFactory();
-			TransactionInterceptor advice = new TransactionInterceptor(transactionManager, PropertiesConverter
-					.stringToProperties("create*=PROPAGATION_REQUIRES_NEW," + isolationLevelForCreate
-							+ "\ngetLastJobExecution*=PROPAGATION_REQUIRES_NEW," + isolationLevelForCreate
-							+ "\n*=PROPAGATION_REQUIRED"));
-			DefaultPointcutAdvisor advisor = new DefaultPointcutAdvisor(advice);
-			NameMatchMethodPointcut pointcut = new NameMatchMethodPointcut();
-			pointcut.addMethodName("*");
-			advisor.setPointcut(pointcut);
-			proxyFactory.addAdvisor(advisor);
+			TransactionInterceptor advice = new TransactionInterceptor(transactionManager,
+					PropertiesConverter.stringToProperties("create*=PROPAGATION_REQUIRES_NEW,"
+							+ isolationLevelForCreate + "\ngetLastJobExecution*=PROPAGATION_REQUIRES_NEW,"
+							+ isolationLevelForCreate + "\n*=PROPAGATION_REQUIRED"));
+			if (validateTransactionState) {
+				DefaultPointcutAdvisor advisor = new DefaultPointcutAdvisor(new MethodInterceptor() {
+					public Object invoke(MethodInvocation invocation) throws Throwable {
+						if (TransactionSynchronizationManager.isActualTransactionActive()) {
+							throw new IllegalStateException(
+									"Existing transaction detected in JobRepository. "
+											+ "Please fix this and try again (e.g. remove @Transactional annotations from client).");
+						}
+						return invocation.proceed();
+					}
+				});
+				NameMatchMethodPointcut pointcut = new NameMatchMethodPointcut();
+				pointcut.addMethodName("create*");
+				advisor.setPointcut(pointcut);
+				proxyFactory.addAdvisor(advisor);
+			}
+			proxyFactory.addAdvice(advice);
 			proxyFactory.setProxyTargetClass(false);
 			proxyFactory.addInterface(JobRepository.class);
 			proxyFactory.setTarget(getTarget());

spring-batch-core/src/main/java/org/springframework/batch/core/repository/support/JobRepositoryFactoryBean.java

@@ -53,6 +53,7 @@ import org.springframework.util.StringUtils;
  * 
  * @author Ben Hale
  * @author Lucas Ward
+ * @author Dave Syer
  */
 public class JobRepositoryFactoryBean extends AbstractJobRepositoryFactoryBean implements InitializingBean {
 

spring-batch-core/src/test/java/org/springframework/batch/core/repository/support/SimpleJobRepositoryProxyTests.java

@@ -13,6 +13,7 @@ import org.springframework.batch.core.JobParameters;
 import org.springframework.batch.core.job.JobSupport;
 import org.springframework.batch.core.repository.JobRepository;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.test.annotation.DirtiesContext;
 import org.springframework.test.context.ContextConfiguration;
 import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
 import org.springframework.transaction.annotation.Transactional;
@@ -35,8 +36,18 @@ public class SimpleJobRepositoryProxyTests {
 	private JobSupport job = new JobSupport("SimpleJobRepositoryProxyTestsJob");
 
 	@Transactional
+	@Test(expected=IllegalStateException.class)
+	@DirtiesContext
+	public void testCreateAndFindWithExistingTransaction() throws Exception {
+		assertFalse(advice.invoked);
+		JobExecution jobExecution = jobRepository.createJobExecution(job.getName(), new JobParameters());
+		assertNotNull(jobExecution);
+		assertTrue(advice.invoked);
+	}
+
 	@Test
-	public void testCreateAndFind() throws Exception {
+	@DirtiesContext
+	public void testCreateAndFindNoTransaction() throws Exception {
 		assertFalse(advice.invoked);
 		JobExecution jobExecution = jobRepository.createJobExecution(job.getName(), new JobParameters());
 		assertNotNull(jobExecution);