BATCH-1181:

* Added test to make sure <step/> element's parent= attribute could reference a beans:bean
 * Some small cleanup

spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/ChunkElementParser.java

@@ -60,7 +60,7 @@ public class ChunkElementParser {
 
 		MutablePropertyValues propertyValues = bd.getPropertyValues();
 
-		propertyValues.addPropertyValue("hasChunkTaskletElement", Boolean.TRUE);
+		propertyValues.addPropertyValue("hasChunkElement", Boolean.TRUE);
 
 		String readerBeanId = element.getAttribute("reader");
 		if (StringUtils.hasText(readerBeanId)) {

spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/StepParserStepFactoryBean.java

@@ -65,6 +65,9 @@ class StepParserStepFactoryBean<I, O> implements FactoryBean, BeanNameAware {
 	//
 	private String name;
 
+	//
+	// Tasklet Attributes
+	//
 	private Boolean allowStartIfComplete;
 
 	private JobRepository jobRepository;
@@ -76,10 +79,12 @@ class StepParserStepFactoryBean<I, O> implements FactoryBean, BeanNameAware {
 	private PlatformTransactionManager transactionManager;
 
 	//
-	// Step Elements
+	// Tasklet Elements
 	//
 	private StepListener[] listeners;
 
+	private Collection<Class<? extends Throwable>> noRollbackExceptionClasses;
+
 	private int transactionTimeout = DefaultTransactionAttribute.TIMEOUT_DEFAULT;
 
 	private Propagation propagation;
@@ -87,7 +92,7 @@ class StepParserStepFactoryBean<I, O> implements FactoryBean, BeanNameAware {
 	private Isolation isolation;
 
 	//
-	// Tasklet Attributes
+	// Chunk Attributes
 	//
 	private Integer cacheCapacity;
 
@@ -110,7 +115,7 @@ class StepParserStepFactoryBean<I, O> implements FactoryBean, BeanNameAware {
 	private ItemWriter<? super O> itemWriter;
 
 	//
-	// Tasklet Elements
+	// Chunk Elements
 	//
 	private RetryListener[] retryListeners;
 
@@ -120,14 +125,12 @@ class StepParserStepFactoryBean<I, O> implements FactoryBean, BeanNameAware {
 
 	private Collection<Class<? extends Throwable>> fatalExceptionClasses;
 
-	private Collection<Class<? extends Throwable>> noRollbackExceptionClasses;
-
 	private ItemStream[] streams;
 
 	//
 	// Additional
 	//
-	private boolean hasChunkTaskletElement = false;
+	private boolean hasChunkElement = false;
 
 	/**
 	 * Create a {@link Step} from the configuration provided.
@@ -135,7 +138,7 @@ class StepParserStepFactoryBean<I, O> implements FactoryBean, BeanNameAware {
 	 * @see FactoryBean#getObject()
 	 */
 	public final Object getObject() throws Exception {
-		if (hasChunkTaskletElement) {
+		if (hasChunkElement) {
 			Assert.isNull(tasklet, "Step [" + name
 					+ "] has both a <chunk/> element and a 'ref' attribute  referencing a Tasklet.");
 
@@ -300,7 +303,6 @@ class StepParserStepFactoryBean<I, O> implements FactoryBean, BeanNameAware {
 				new PropertyNamePair(retryListeners, "retry-listeners"),
 				new PropertyNamePair(skippableExceptionClasses, "skippable-exception-classes"),
 				new PropertyNamePair(retryableExceptionClasses, "retryable-exception-classes"),
-				new PropertyNamePair(noRollbackExceptionClasses, "no-rollback-exception-classes"),
 				new PropertyNamePair(fatalExceptionClasses, "fatal-exception-classes") };
 
 		List<String> wrong = new ArrayList<String>();
@@ -374,6 +376,10 @@ class StepParserStepFactoryBean<I, O> implements FactoryBean, BeanNameAware {
 		}
 	}
 
+	// =========================================================
+	// Tasklet Attributes
+	// =========================================================
+
 	/**
 	 * Public setter for the flag to indicate that the step should be replayed
 	 * on a restart, even if successful the first time.
@@ -420,7 +426,7 @@ class StepParserStepFactoryBean<I, O> implements FactoryBean, BeanNameAware {
 	}
 
 	// =========================================================
-	// Step Elements
+	// Tasklet Elements
 	// =========================================================
 
 	/**
@@ -434,6 +440,16 @@ class StepParserStepFactoryBean<I, O> implements FactoryBean, BeanNameAware {
 		this.listeners = listeners;
 	}
 
+	/**
+	 * Exception classes that may not cause a rollback if encountered in the
+	 * right place.
+	 * 
+	 * @param noRollbackExceptionClasses the noRollbackExceptionClasses to set
+	 */
+	public void setNoRollbackExceptionClasses(Collection<Class<? extends Throwable>> noRollbackExceptionClasses) {
+		this.noRollbackExceptionClasses = noRollbackExceptionClasses;
+	}
+
 	/**
 	 * @param transactionTimeout the transactionTimeout to set
 	 */
@@ -456,7 +472,7 @@ class StepParserStepFactoryBean<I, O> implements FactoryBean, BeanNameAware {
 	}
 
 	// =========================================================
-	// Tasklet Attributes
+	// Chunk Attributes
 	// =========================================================
 
 	/**
@@ -566,7 +582,7 @@ class StepParserStepFactoryBean<I, O> implements FactoryBean, BeanNameAware {
 	}
 
 	// =========================================================
-	// Tasklet Elements
+	// Chunk Elements
 	// =========================================================
 
 	/**
@@ -598,16 +614,6 @@ class StepParserStepFactoryBean<I, O> implements FactoryBean, BeanNameAware {
 		this.retryableExceptionClasses = retryableExceptionClasses;
 	}
 
-	/**
-	 * Exception classes that may not cause a rollback if encountered in the
-	 * right place.
-	 * 
-	 * @param noRollbackExceptionClasses the noRollbackExceptionClasses to set
-	 */
-	public void setNoRollbackExceptionClasses(Collection<Class<? extends Throwable>> noRollbackExceptionClasses) {
-		this.noRollbackExceptionClasses = noRollbackExceptionClasses;
-	}
-
 	/**
 	 * Public setter for exception classes that should cause immediate failure.
 	 * 
@@ -628,10 +634,14 @@ class StepParserStepFactoryBean<I, O> implements FactoryBean, BeanNameAware {
 		this.streams = streams;
 	}
 
+	// =========================================================
+	// Additional
+	// =========================================================
+
 	/**
-	 * @param hasChunkTaskletElement
+	 * @param hasChunkElement
 	 */
-	public void setHasChunkTaskletElement(boolean hasChunkTaskletElement) {
-		this.hasChunkTaskletElement = hasChunkTaskletElement;
+	public void setHasChunkElement(boolean hasChunkElement) {
+		this.hasChunkElement = hasChunkElement;
 	}
 }

spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/StepParserStepFactoryBeanTests.java

@@ -146,7 +146,7 @@ public class StepParserStepFactoryBeanTests {
 	@Test
 	public void testSimpleStep() throws Exception {
 		StepParserStepFactoryBean<Object, Object> fb = new StepParserStepFactoryBean<Object, Object>();
-		fb.setHasChunkTaskletElement(true);
+		fb.setHasChunkElement(true);
 		fb.setBeanName("step1");
 		fb.setAllowStartIfComplete(true);
 		fb.setJobRepository(new JobRepositorySupport());
@@ -172,7 +172,7 @@ public class StepParserStepFactoryBeanTests {
 	@Test
 	public void testFaultTolerantStep() throws Exception {
 		StepParserStepFactoryBean<Object, Object> fb = new StepParserStepFactoryBean<Object, Object>();
-		fb.setHasChunkTaskletElement(true);
+		fb.setHasChunkElement(true);
 		fb.setBeanName("step1");
 		fb.setAllowStartIfComplete(true);
 		fb.setJobRepository(new JobRepositorySupport());

spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/StepParserTests.java

@@ -26,6 +26,7 @@ import org.springframework.aop.framework.Advised;
 import org.springframework.batch.core.Step;
 import org.springframework.batch.core.StepExecutionListener;
 import org.springframework.batch.core.listener.StepExecutionListenerSupport;
+import org.springframework.batch.core.step.tasklet.Tasklet;
 import org.springframework.batch.core.step.tasklet.TaskletStep;
 import org.springframework.batch.repeat.CompletionPolicy;
 import org.springframework.batch.repeat.policy.SimpleCompletionPolicy;
@@ -209,4 +210,23 @@ public class StepParserTests {
 		return txa;
 	}
 
+	@Test
+	public void testInheritFromBean() throws Exception {
+		ApplicationContext ctx = new ClassPathXmlApplicationContext(
+				"org/springframework/batch/core/configuration/xml/StepParserParentAttributeTests-context.xml");
+		
+		assertTrue(getTasklet("s9", ctx) instanceof DummyTasklet);
+		assertTrue(getTasklet("s10", ctx) instanceof DummyTasklet);
+	}
+
+	@SuppressWarnings("unchecked")
+	private Tasklet getTasklet(String stepName, ApplicationContext ctx) {
+		Map<String, Object> beans = ctx.getBeansOfType(Step.class);
+		assertTrue(beans.containsKey(stepName));
+		Step step = (Step) ctx.getBean(stepName);
+		assertTrue(step instanceof TaskletStep);
+		Object tasklet = ReflectionTestUtils.getField(step, "tasklet");
+		assertTrue(tasklet instanceof Tasklet);
+		return (Tasklet)tasklet;
+	}
 }

spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/StepParserParentAttributeTests-context.xml

@@ -32,7 +32,9 @@
 				<transaction-attributes propagation="REQUIRED" isolation="DEFAULT" />
 			</tasklet>
 		</step>
-		<step id="s8" parent="standalone8" next="s5" />
+		<step id="s8" parent="standalone8" next="s9" />
+		<step id="s9" parent="standalone9" next="s10" />
+		<step id="s10" parent="standalone10" />
 	</job>
 
 	<step id="standalone2" parent="baseStep">
@@ -61,6 +63,18 @@
 		</tasklet>
 	</step>
 
+	<beans:bean id="standalone9" class="org.springframework.batch.core.step.tasklet.TaskletStep">
+		<beans:property name="tasklet" ref="dummyTasklet"/>
+		<beans:property name="jobRepository" ref="jobRepository"/>
+		<beans:property name="transactionManager" ref="transactionManager"/>
+	</beans:bean>
+
+	<beans:bean id="standalone10" class="org.springframework.batch.core.step.tasklet.TaskletStep" abstract="true">
+		<beans:property name="tasklet" ref="dummyTasklet"/>
+		<beans:property name="jobRepository" ref="jobRepository"/>
+		<beans:property name="transactionManager" ref="transactionManager"/>
+	</beans:bean>
+
 	<step id="baseStep" abstract="true">
 		<tasklet>
 			<transaction-attributes timeout="10"/>