RESOLVED - issue BATCH-180: JobConfigurations do not get their names set if they are child beans

http://opensource.atlassian.com/projects/spring/browse/BATCH-180

Removed setName() from *Configuration - you have to use the bean id or setBeanName().

core/src/main/java/org/springframework/batch/core/configuration/JobConfiguration.java

@@ -35,9 +35,9 @@ public class JobConfiguration implements BeanNameAware {
 	private List stepConfigurations = new ArrayList();
 
 	private String name;
-	
+
 	private boolean restartable = false;
-	
+
 	private int startLimit = Integer.MAX_VALUE;
 
 	/**
@@ -50,6 +50,7 @@ public class JobConfiguration implements BeanNameAware {
 	/**
 	 * Convenience constructor to immediately add name (which is mandatory but
 	 * not final).
+	 * 
 	 * @param name
 	 */
 	public JobConfiguration(String name) {
@@ -57,20 +58,20 @@ public class JobConfiguration implements BeanNameAware {
 		this.name = name;
 	}
 
+	/**
+	 * The callback from {@link BeanNameAware} comes after the setters, so it
+	 * will always overwrite the name with the bean id.
+	 * 
+	 * @see org.springframework.beans.factory.BeanNameAware#setBeanName(java.lang.String)
+	 */
 	public void setBeanName(String name) {
-		if (this.name == null) {
-			this.name = name;
-		}
+		this.name = name;
 	}
 
 	public String getName() {
 		return name;
 	}
 
-	public void setName(String name) {
-		this.name = name;
-	}
-
 	public List getStepConfigurations() {
 		return stepConfigurations;
 	}
@@ -83,19 +84,19 @@ public class JobConfiguration implements BeanNameAware {
 	public void addStep(StepConfiguration stepConfiguration) {
 		this.stepConfigurations.add(stepConfiguration);
 	}
-	
+
 	public int getStartLimit() {
 		return startLimit;
 	}
-	
+
 	public void setStartLimit(int startLimit) {
 		this.startLimit = startLimit;
 	}
-	
+
 	public void setRestartable(boolean restartable) {
 		this.restartable = restartable;
 	}
-	
+
 	public boolean isRestartable() {
 		return restartable;
 	}

core/src/main/java/org/springframework/batch/core/configuration/StepConfigurationSupport.java

@@ -16,6 +16,7 @@
 package org.springframework.batch.core.configuration;
 
 import org.springframework.batch.core.tasklet.Tasklet;
+import org.springframework.beans.factory.BeanNameAware;
 
 /**
  * Basic no-op support implementation for use as base class for
@@ -24,21 +25,22 @@ import org.springframework.batch.core.tasklet.Tasklet;
  * @author Dave Syer
  * 
  */
-public class StepConfigurationSupport implements StepConfiguration {
+public class StepConfigurationSupport implements StepConfiguration,
+		BeanNameAware {
 
 	private String name;
 	private int startLimit = Integer.MAX_VALUE;
 	private Tasklet tasklet;
 	private boolean allowStartIfComplete;
 	private boolean saveRestartData = false;
-	
+
 	/**
 	 * Default constructor for {@link StepConfigurationSupport}.
 	 */
 	public StepConfigurationSupport() {
 		super();
 	}
-	
+
 	/**
 	 * @param string
 	 */
@@ -49,33 +51,37 @@ public class StepConfigurationSupport implements StepConfiguration {
 
 	/*
 	 * (non-Javadoc)
+	 * 
 	 * @see org.springframework.batch.core.configuration.StepConfiguration#getName()
 	 */
 	public String getName() {
 		return this.name;
 	}
-	
+
 	/**
-	 * Public setter for the name.
-	 *
-	 * @param name the name to set
+	 * Set the name property. Always overrides the default value if this
+	 * object is a Spring bean.
+	 * 
+	 * @see org.springframework.beans.factory.BeanNameAware#setBeanName(java.lang.String)
 	 */
-	public void setName(String name) {
+	public void setBeanName(String name) {
 		this.name = name;
 	}
 
 	/*
 	 * (non-Javadoc)
+	 * 
 	 * @see org.springframework.batch.core.configuration.StepConfiguration#getStartLimit()
 	 */
 	public int getStartLimit() {
 		return this.startLimit;
 	}
-	
+
 	/**
 	 * Public setter for the startLimit.
-	 *
-	 * @param startLimit the startLimit to set
+	 * 
+	 * @param startLimit
+	 *            the startLimit to set
 	 */
 	public void setStartLimit(int startLimit) {
 		this.startLimit = startLimit;
@@ -83,16 +89,18 @@ public class StepConfigurationSupport implements StepConfiguration {
 
 	/*
 	 * (non-Javadoc)
+	 * 
 	 * @see org.springframework.batch.core.configuration.StepConfiguration#getTasklet()
 	 */
 	public Tasklet getTasklet() {
 		return this.tasklet;
 	}
-	
+
 	/**
 	 * Public setter for the tasklet.
-	 *
-	 * @param tasklet the tasklet to set
+	 * 
+	 * @param tasklet
+	 *            the tasklet to set
 	 */
 	public void setTasklet(Tasklet tasklet) {
 		this.tasklet = tasklet;
@@ -100,16 +108,18 @@ public class StepConfigurationSupport implements StepConfiguration {
 
 	/*
 	 * (non-Javadoc)
+	 * 
 	 * @see org.springframework.batch.core.configuration.StepConfiguration#shouldAllowStartIfComplete()
 	 */
 	public boolean isAllowStartIfComplete() {
 		return this.allowStartIfComplete;
 	}
-	
+
 	/**
 	 * Public setter for the shouldAllowStartIfComplete.
-	 *
-	 * @param allowStartIfComplete the shouldAllowStartIfComplete to set
+	 * 
+	 * @param allowStartIfComplete
+	 *            the shouldAllowStartIfComplete to set
 	 */
 	public void setAllowStartIfComplete(boolean allowStartIfComplete) {
 		this.allowStartIfComplete = allowStartIfComplete;

core/src/test/java/org/springframework/batch/core/configuration/JobConfigurationTests.java

@@ -42,7 +42,7 @@ public class JobConfigurationTests extends TestCase {
 	 */
 	public void testSetBeanName() {
 		configuration.setBeanName("foo");
-		assertEquals("job", configuration.getName());
+		assertEquals("foo", configuration.getName());
 	}
 
 	/**
@@ -50,21 +50,12 @@ public class JobConfigurationTests extends TestCase {
 	 * {@link org.springframework.batch.core.configuration.JobConfiguration#setBeanName(java.lang.String)}.
 	 */
 	public void testSetBeanNameWithNullName() {
-		configuration.setName(null);
+		configuration = new JobConfiguration(null);
 		assertEquals(null, configuration.getName());
 		configuration.setBeanName("foo");
 		assertEquals("foo", configuration.getName());
 	}
 
-	/**
-	 * Test method for
-	 * {@link org.springframework.batch.core.configuration.JobConfiguration#setName(java.lang.String)}.
-	 */
-	public void testSetName() {
-		configuration.setName("foo");
-		assertEquals("foo", configuration.getName());
-	}
-
 	/**
 	 * Test method for
 	 * {@link org.springframework.batch.core.configuration.JobConfiguration#setSteps(java.util.List)}.

core/src/test/java/org/springframework/batch/core/configuration/SpringBeanJobConfigurationTests.java

@@ -0,0 +1,69 @@
+/*
+ * Copyright 2006-2007 the original author or authors.
+ *
+ * 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.springframework.batch.core.configuration;
+
+import junit.framework.TestCase;
+
+import org.springframework.beans.factory.config.ConstructorArgumentValues;
+import org.springframework.beans.factory.support.ChildBeanDefinition;
+import org.springframework.beans.factory.support.RootBeanDefinition;
+import org.springframework.context.support.GenericApplicationContext;
+import org.springframework.context.support.StaticApplicationContext;
+
+public class SpringBeanJobConfigurationTests extends TestCase {
+
+	public void testBeanName() throws Exception {
+		StaticApplicationContext context = new StaticApplicationContext();
+		JobConfiguration configuration = new JobConfiguration();
+		context.getAutowireCapableBeanFactory().initializeBean(configuration,
+				"bean");
+		assertNotNull(configuration.getName());
+		configuration.setBeanName("foo");
+		context.getAutowireCapableBeanFactory().initializeBean(configuration,
+				"bean");
+		assertEquals("bean", configuration.getName());
+	}
+
+	public void testBeanNameWithBeanDefinition() throws Exception {
+		GenericApplicationContext context = new GenericApplicationContext();
+		ConstructorArgumentValues args = new ConstructorArgumentValues();
+		args.addGenericArgumentValue("foo");
+		context.registerBeanDefinition("bean", new RootBeanDefinition(
+				JobConfiguration.class, args, null));
+		JobConfiguration configuration = (JobConfiguration) context
+				.getBean("bean");
+		assertNotNull(configuration.getName());
+		assertEquals("bean", configuration.getName());
+		configuration.setBeanName("foo");
+		assertEquals("foo", configuration.getName());
+	}
+
+	public void testBeanNameWithParentBeanDefinition() throws Exception {
+		GenericApplicationContext context = new GenericApplicationContext();
+		ConstructorArgumentValues args = new ConstructorArgumentValues();
+		args.addGenericArgumentValue("bar");
+		context.registerBeanDefinition("parent", new RootBeanDefinition(
+				JobConfiguration.class, args, null));
+		context.registerBeanDefinition("bean", new ChildBeanDefinition("parent"));
+		JobConfiguration configuration = (JobConfiguration) context
+				.getBean("bean");
+		assertNotNull(configuration.getName());
+		assertEquals("bean", configuration.getName());
+		configuration.setBeanName("foo");
+		assertEquals("foo", configuration.getName());
+	}
+}

core/src/test/java/org/springframework/batch/core/configuration/StepConfigurationSupportTests.java

@@ -40,8 +40,7 @@ public class StepConfigurationSupportTests extends TestCase {
 	 * Test method for {@link org.springframework.batch.core.configuration.StepConfigurationSupport#getName()}.
 	 */
 	public void testGetName() {
-		configuration.setName("foo");
-		assertEquals("foo", configuration.getName());
+		assertEquals("step", configuration.getName());
 	}
 
 	/**