From 945c0d91df216feeeb1528c079caf4b8ba2f20d6 Mon Sep 17 00:00:00 2001
From: dsyer <dsyer@2e8e966d-6415-0410-94b9-dc4e65435d03>
Date: Wed, 25 Mar 2009 09:15:02 +0000
Subject: [PATCH] BATCH-1095, BATCH-1174: Late binding of jobParameters does
 not work if late binding expression is not preceded or trailed by string

---
 .../scope/util/PlaceholderTargetSource.java   | 103 +++++++----
 .../PlaceholderTargetSourceErrorTests.java    | 173 ++++++++++++++++++
 ...ceholderTargetSourceErrorTests-context.xml |  42 +++++
 .../PlaceholderTargetSourceTests-context.xml  |   7 +-
 .../src/main/resources/batch-hsql.properties  |   1 +
 .../src/main/resources/jobs/restartSample.xml |  33 ++--
 .../batch/sample/RestartFunctionalTests.java  |  13 +-
 7 files changed, 306 insertions(+), 66 deletions(-)
 create mode 100644 spring-batch-core/src/test/java/org/springframework/batch/core/scope/util/PlaceholderTargetSourceErrorTests.java
 create mode 100644 spring-batch-core/src/test/resources/org/springframework/batch/core/scope/util/PlaceholderTargetSourceErrorTests-context.xml

diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/scope/util/PlaceholderTargetSource.java b/spring-batch-core/src/main/java/org/springframework/batch/core/scope/util/PlaceholderTargetSource.java
index 28092993d..dba139d81 100644
--- a/spring-batch-core/src/main/java/org/springframework/batch/core/scope/util/PlaceholderTargetSource.java
+++ b/spring-batch-core/src/main/java/org/springframework/batch/core/scope/util/PlaceholderTargetSource.java
@@ -106,37 +106,24 @@ public class PlaceholderTargetSource extends SimpleBeanTargetSource implements I
 					String key = (String) value;
 					if (key.startsWith(PLACEHOLDER_PREFIX) && key.endsWith(PLACEHOLDER_SUFFIX)) {
 						key = extractKey(key);
-						result = convertFromContext(key, requiredType);
+						result = convertFromContext(key, requiredType, typeConverter);
+						if (result==null) {
+							Object property = getPropertyFromContext(key);
+							// Give the normal type converter a chance by reversing to a String
+							if (property!=null) {
+								property = convertToString(property, typeConverter);
+								if (property!=null) {
+									value = property;
+								}
+							}
+						}
 					}
 				}
 				else if (requiredType.isAssignableFrom(value.getClass())) {
 					result = value;
 				}
 				else if (requiredType.isAssignableFrom(String.class)) {
-					if (typeConverter instanceof PropertyEditorRegistrySupport) {
-						/*
-						 * PropertyEditorRegistrySupport is de rigeur with
-						 * TypeConverter instances used internally by Spring. If
-						 * we have one of those then we can convert to String
-						 * but the TypeConverter doesn't know how to.
-						 */
-						PropertyEditorRegistrySupport registry = (PropertyEditorRegistrySupport) typeConverter;
-						PropertyEditor editor = registry.findCustomEditor(value.getClass(), null);
-						if (editor != null) {
-							if (registry.isSharedEditor(editor)) {
-								// Synchronized access to shared editor
-								// instance.
-								synchronized (editor) {
-									editor.setValue(value);
-									result = editor.getAsText();
-								}
-							}
-							else {
-								editor.setValue(value);
-								result = editor.getAsText();
-							}
-						}
-					}
+					result = convertToString(value, typeConverter);
 					if (result == null) {
 						logger.debug("Falling back on toString for conversion of : [" + value.getClass() + "]");
 						result = value.toString();
@@ -216,21 +203,67 @@ public class PlaceholderTargetSource extends SimpleBeanTargetSource implements I
 
 	/**
 	 * @param value
-	 * @param requiredType
-	 * @return
+	 * @param typeConverter
+	 * @return a String representation of the input if possible
 	 */
-	private Object convertFromContext(String key, Class<?> requiredType) {
-		Object result = null;
-		BeanWrapper wrapper = new BeanWrapperImpl(contextFactory.getContext());
-		if (wrapper.isReadableProperty(key)) {
-			Object property = wrapper.getPropertyValue(key);
-			if (property == null || requiredType.isAssignableFrom(property.getClass())) {
-				result = property;
+	protected String convertToString(Object value, TypeConverter typeConverter) {
+		String result = null;
+		try {
+			// Give it one chance to convert - this forces the default editors to be registered
+			result = (String) typeConverter.convertIfNecessary(value, String.class);
+		} catch (TypeMismatchException e) {
+			// ignore
+		}
+		if (result== null && typeConverter instanceof PropertyEditorRegistrySupport) {
+			/*
+			 * PropertyEditorRegistrySupport is de rigeur with TypeConverter
+			 * instances used internally by Spring. If we have one of those then
+			 * we can convert to String but the TypeConverter doesn't know how
+			 * to.
+			 */
+			PropertyEditorRegistrySupport registry = (PropertyEditorRegistrySupport) typeConverter;
+			PropertyEditor editor = registry.findCustomEditor(value.getClass(), null);
+			if (editor != null) {
+				if (registry.isSharedEditor(editor)) {
+					// Synchronized access to shared editor
+					// instance.
+					synchronized (editor) {
+						editor.setValue(value);
+						result = editor.getAsText();
+					}
+				}
+				else {
+					editor.setValue(value);
+					result = editor.getAsText();
+				}
 			}
 		}
 		return result;
 	}
 
+	/**
+	 * @param value
+	 * @param requiredType
+	 * @param typeConverter
+	 * @return
+	 */
+	private Object convertFromContext(String key, Class<?> requiredType, TypeConverter typeConverter) {
+		Object result = null;
+		Object property = getPropertyFromContext(key);
+		if (property == null || requiredType.isAssignableFrom(property.getClass())) {
+			result = property;
+		}
+		return result;
+	}
+
+	private Object getPropertyFromContext(String key) {
+		BeanWrapper wrapper = new BeanWrapperImpl(contextFactory.getContext());
+		if (wrapper.isReadableProperty(key)) {
+			return wrapper.getPropertyValue(key);
+		}
+		return null;
+	}
+
 	private String extractKey(String value) {
 		if (value.startsWith(PLACEHOLDER_PREFIX)) {
 			value = value.substring(PLACEHOLDER_PREFIX.length());
@@ -276,7 +309,7 @@ public class PlaceholderTargetSource extends SimpleBeanTargetSource implements I
 
 	private void replaceIfTypeMatches(StringBuilder result, int first, int next, String key, Class<?> requiredType,
 			TypeConverter typeConverter) {
-		Object property = convertFromContext(key, requiredType);
+		Object property = convertFromContext(key, requiredType, typeConverter);
 		if (property != null) {
 			result.replace(first, next + 1, (String) typeConverter.convertIfNecessary(property, String.class));
 		}
diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/scope/util/PlaceholderTargetSourceErrorTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/scope/util/PlaceholderTargetSourceErrorTests.java
new file mode 100644
index 000000000..5781f0bf5
--- /dev/null
+++ b/spring-batch-core/src/test/java/org/springframework/batch/core/scope/util/PlaceholderTargetSourceErrorTests.java
@@ -0,0 +1,173 @@
+package org.springframework.batch.core.scope.util;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import java.util.Collections;
+import java.util.Date;
+import java.util.Map;
+
+import org.apache.commons.io.IOUtils;
+import org.junit.Test;
+import org.springframework.beans.factory.xml.XmlBeanDefinitionReader;
+import org.springframework.context.support.GenericApplicationContext;
+import org.springframework.core.io.ByteArrayResource;
+import org.springframework.core.io.ClassPathResource;
+import org.springframework.core.io.Resource;
+
+public class PlaceholderTargetSourceErrorTests extends ContextFactorySupport {
+
+	private Map<String, Object> map = Collections.singletonMap("foo.foo", (Object) "bar");
+
+	private Date date = new Date(1L);
+
+	public Object getContext() {
+		return this;
+	}
+
+	public String getFoo() {
+		return "bar";
+	}
+
+	public Map<String, Object> getMap() {
+		return map;
+	}
+
+	public Node getParent() {
+		return new Foo("spam");
+	}
+
+	public Long getLong() {
+		return 12345678912345L;
+	}
+
+	public Integer getInteger() {
+		return 4321;
+	}
+
+	public Date getDate() {
+		return date;
+	}
+
+	public String getGarbage() {
+		return null;
+	}
+
+	private PlaceholderTargetSource createValue(String name, String value) throws Exception {
+		String input = IOUtils.toString(new ClassPathResource(getClass().getSimpleName() + "-context.xml", getClass())
+				.getInputStream());
+		input = input.replace("<!-- INSERT -->", String.format("<property name=\"%s\" value=\"%s\" />", name, value));
+		Resource resource = new ByteArrayResource(input.getBytes());
+		GenericApplicationContext context = new GenericApplicationContext();
+		new XmlBeanDefinitionReader(context).loadBeanDefinitions(resource);
+		context.refresh();
+		// XmlBeanFactory context = new XmlBeanFactory(resource);
+		return (PlaceholderTargetSource) context.getBean("value");
+	}
+
+	@Test
+	public void testPartialReplaceSunnyDay() throws Exception {
+		Node target = (Node) createValue("name", "#{foo}-bar").getTarget();
+		assertEquals("bar-bar", target.getName());
+	}
+
+	@Test
+	public void testFullReplaceSunnyDay() throws Exception {
+		Node target = (Node) createValue("name", "#{foo}").getTarget();
+		assertEquals("bar", target.getName());
+	}
+
+	@Test
+	public void testPartialReplaceIntegerToString() throws Exception {
+		Node target = (Node) createValue("name", "foo-#{integer}").getTarget();
+		assertEquals("foo-4321", target.getName());
+	}
+
+	@Test
+	public void testFullReplaceIntegerToString() throws Exception {
+		Node target = (Node) createValue("name", "#{integer}").getTarget();
+		assertEquals("4321", target.getName());
+	}
+
+	@Test
+	public void testFullReplaceIntegerToLong() throws Exception {
+		Node target = (Node) createValue("value", "#{integer}").getTarget();
+		assertEquals(4321L, target.getValue());
+	}
+
+	@Test
+	public void testFullReplaceIntegerToNode() throws Exception {
+		try {
+			Node target = (Node) createValue("parent", "#{integer}").getTarget();
+			assertEquals("4321", target.getParent());
+			fail("Expected IllegalArgumentException");
+		}
+		catch (Exception e) {
+			String message = e.getMessage();
+			assertTrue("Wrong message: " + message, message.toLowerCase().contains("cannot convert"));
+		}
+	}
+
+	public static interface Node {
+		String getName();
+
+		Date getDate();
+
+		Node getParent();
+
+		long getValue();
+	}
+
+	public static class Foo implements Node {
+
+		private String name;
+
+		private Date date;
+
+		private Node parent;
+
+		private long value;
+
+		public Foo() {
+		}
+
+		public long getValue() {
+			return value;
+		}
+
+		public void setValue(long value) {
+			this.value = value;
+		}
+
+		public Foo(String name) {
+			this.name = name;
+		}
+
+		public String getName() {
+			return name;
+		}
+
+		public Date getDate() {
+			return date;
+		}
+
+		public void setDate(Date date) {
+			this.date = date;
+		}
+
+		public void setName(String name) {
+			this.name = name;
+		}
+
+		public Node getParent() {
+			return parent;
+		}
+
+		public void setParent(Node parent) {
+			this.parent = parent;
+		}
+
+	}
+
+}
diff --git a/spring-batch-core/src/test/resources/org/springframework/batch/core/scope/util/PlaceholderTargetSourceErrorTests-context.xml b/spring-batch-core/src/test/resources/org/springframework/batch/core/scope/util/PlaceholderTargetSourceErrorTests-context.xml
new file mode 100644
index 000000000..44b47ea6b
--- /dev/null
+++ b/spring-batch-core/src/test/resources/org/springframework/batch/core/scope/util/PlaceholderTargetSourceErrorTests-context.xml
@@ -0,0 +1,42 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<beans xmlns="http://www.springframework.org/schema/beans"
+	xmlns:aop="http://www.springframework.org/schema/aop" xmlns:tx="http://www.springframework.org/schema/tx"
+	xmlns:p="http://www.springframework.org/schema/p" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+	xsi:schemaLocation="
+		http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans.xsd
+		http://www.springframework.org/schema/aop http://www.springframework.org/schema/aop/spring-aop.xsd
+		http://www.springframework.org/schema/tx http://www.springframework.org/schema/tx/spring-tx.xsd">
+
+	<bean id="context"
+		class="org.springframework.batch.core.scope.util.PlaceholderTargetSourceTests" />
+
+	<bean id="value"
+		class="org.springframework.batch.core.scope.util.PlaceholderTargetSource">
+		<property name="contextFactory" ref="context" />
+		<property name="targetBeanName" value="target" />
+	</bean>
+
+	<bean id="target"
+		class="org.springframework.batch.core.scope.util.PlaceholderTargetSourceErrorTests$Foo"
+		lazy-init="true">
+		<!-- INSERT -->
+	</bean>
+
+	<bean class="org.springframework.beans.factory.config.CustomEditorConfigurer">
+		<property name="customEditors">
+			<map>
+				<entry key="java.util.Date">
+					<bean class="org.springframework.beans.propertyeditors.CustomDateEditor">
+						<constructor-arg>
+							<bean class="java.text.SimpleDateFormat">
+								<constructor-arg value="yyyy/MM/dd" />
+							</bean>
+						</constructor-arg>
+						<constructor-arg value="false"/>
+					</bean>
+				</entry>
+			</map>
+		</property>
+	</bean>
+
+</beans>
\ No newline at end of file
diff --git a/spring-batch-core/src/test/resources/org/springframework/batch/core/scope/util/PlaceholderTargetSourceTests-context.xml b/spring-batch-core/src/test/resources/org/springframework/batch/core/scope/util/PlaceholderTargetSourceTests-context.xml
index 6e4089230..e5a5691bf 100644
--- a/spring-batch-core/src/test/resources/org/springframework/batch/core/scope/util/PlaceholderTargetSourceTests-context.xml
+++ b/spring-batch-core/src/test/resources/org/springframework/batch/core/scope/util/PlaceholderTargetSourceTests-context.xml
@@ -6,6 +6,10 @@
 		http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans.xsd
 		http://www.springframework.org/schema/aop http://www.springframework.org/schema/aop/spring-aop.xsd
 		http://www.springframework.org/schema/tx http://www.springframework.org/schema/tx/spring-tx.xsd">
+
+	<bean id="context"
+		class="org.springframework.batch.core.scope.util.PlaceholderTargetSourceTests" />
+
 	<bean id="vanilla"
 		class="org.springframework.batch.core.scope.util.PlaceholderTargetSource">
 		<property name="contextFactory" ref="context" />
@@ -66,9 +70,6 @@
 		<property name="targetBeanName" value="withEmbeddedDateTarget" />
 	</bean>
 
-	<bean id="context"
-		class="org.springframework.batch.core.scope.util.PlaceholderTargetSourceTests" />
-
 	<bean id="bar"
 		class="org.springframework.batch.core.scope.util.PlaceholderTargetSourceTests$Foo">
 		<property name="name" value="foo" />
diff --git a/spring-batch-samples/src/main/resources/batch-hsql.properties b/spring-batch-samples/src/main/resources/batch-hsql.properties
index 7f28d4f4a..d68b8457e 100644
--- a/spring-batch-samples/src/main/resources/batch-hsql.properties
+++ b/spring-batch-samples/src/main/resources/batch-hsql.properties
@@ -12,6 +12,7 @@ batch.jndi.name=
 batch.naming.factory.initial=
 batch.naming.provider.url=
 batch.schema.script=schema-hsqldb.sql
+batch.drop.script=schema-drop-hsqldb.sql
 batch.business.schema.script=business-schema-hsqldb.sql
 batch.database.incrementer.class=org.springframework.jdbc.support.incrementer.HsqlMaxValueIncrementer
 batch.lob.handler.class=org.springframework.jdbc.support.lob.DefaultLobHandler
diff --git a/spring-batch-samples/src/main/resources/jobs/restartSample.xml b/spring-batch-samples/src/main/resources/jobs/restartSample.xml
index 7e6b9d7f0..cec7feb18 100644
--- a/spring-batch-samples/src/main/resources/jobs/restartSample.xml
+++ b/spring-batch-samples/src/main/resources/jobs/restartSample.xml
@@ -1,9 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<beans:beans xmlns="http://www.springframework.org/schema/batch" 
-	xmlns:beans="http://www.springframework.org/schema/beans" 
-	xmlns:aop="http://www.springframework.org/schema/aop"
-	xmlns:tx="http://www.springframework.org/schema/tx"
-	xmlns:p="http://www.springframework.org/schema/p"
+<beans:beans xmlns="http://www.springframework.org/schema/batch" xmlns:beans="http://www.springframework.org/schema/beans"
+	xmlns:aop="http://www.springframework.org/schema/aop" xmlns:tx="http://www.springframework.org/schema/tx" xmlns:p="http://www.springframework.org/schema/p"
 	xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
 	xsi:schemaLocation="
 		http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans-2.0.xsd
@@ -15,7 +12,7 @@
 		<step id="step1">
 			<tasklet reader="itemReader" processor="processor" writer="tradeWriter" commit-interval="2">
 				<streams>
-					<stream ref="fileItemReader"/>
+					<stream ref="fileItemReader" />
 				</streams>
 			</tasklet>
 		</step>
@@ -31,15 +28,12 @@
 		<beans:property name="dao" ref="tradeDao" />
 	</beans:bean>
 
-	<beans:bean id="itemReader"
-		class="org.springframework.batch.sample.support.ExceptionThrowingItemReaderProxy">
+	<beans:bean id="itemReader" class="org.springframework.batch.sample.support.ExceptionThrowingItemReaderProxy">
 		<beans:property name="delegate" ref="fileItemReader" />
 	</beans:bean>
 
-	<beans:bean id="fileItemReader"
-		class="org.springframework.batch.item.file.FlatFileItemReader">
-		<beans:property name="resource"
-			value="classpath:data/fixedLengthImportJob/input/20070122.teststream.ImportTradeDataStep.txt" />
+	<beans:bean id="fileItemReader" class="org.springframework.batch.item.file.FlatFileItemReader" scope="step">
+		<beans:property name="resource" value="#{jobParameters['input.file']}" />
 		<beans:property name="lineMapper">
 			<beans:bean class="org.springframework.batch.item.file.mapping.DefaultLineMapper">
 				<beans:property name="lineTokenizer" ref="fixedFileTokenizer" />
@@ -49,17 +43,14 @@
 		<beans:property name="saveState" value="true" />
 	</beans:bean>
 
-	<beans:bean id="fixedFileTokenizer"
-		class="org.springframework.batch.item.file.transform.FixedLengthTokenizer">
+	<beans:bean id="fixedFileTokenizer" class="org.springframework.batch.item.file.transform.FixedLengthTokenizer">
 		<beans:property name="names" value="ISIN, Quantity, Price, Customer" />
 		<beans:property name="columns" value="1-12, 13-15, 16-20, 21-29" />
 	</beans:bean>
 
-	<beans:bean id="fixedValidator"
-		class="org.springframework.batch.item.validator.SpringValidator">
+	<beans:bean id="fixedValidator" class="org.springframework.batch.item.validator.SpringValidator">
 		<beans:property name="validator">
-			<beans:bean id="tradeValidator"
-				class="org.springmodules.validation.valang.ValangValidator">
+			<beans:bean id="tradeValidator" class="org.springmodules.validation.valang.ValangValidator">
 				<beans:property name="valang">
 					<beans:value>
 						<![CDATA[
@@ -71,8 +62,7 @@
 		</beans:property>
 	</beans:bean>
 
-	<beans:bean id="tradeDao"
-		class="org.springframework.batch.sample.domain.trade.internal.JdbcTradeDao">
+	<beans:bean id="tradeDao" class="org.springframework.batch.sample.domain.trade.internal.JdbcTradeDao">
 		<beans:property name="dataSource" ref="dataSource" />
 		<beans:property name="incrementer">
 			<beans:bean parent="incrementerParent">
@@ -82,7 +72,6 @@
 		</beans:property>
 	</beans:bean>
 
-	<beans:bean id="fieldSetMapper"
-		class="org.springframework.batch.sample.domain.trade.internal.TradeFieldSetMapper" />
+	<beans:bean id="fieldSetMapper" class="org.springframework.batch.sample.domain.trade.internal.TradeFieldSetMapper" />
 
 </beans:beans>
\ No newline at end of file
diff --git a/spring-batch-samples/src/test/java/org/springframework/batch/sample/RestartFunctionalTests.java b/spring-batch-samples/src/test/java/org/springframework/batch/sample/RestartFunctionalTests.java
index faa228716..9d4a167e3 100644
--- a/spring-batch-samples/src/test/java/org/springframework/batch/sample/RestartFunctionalTests.java
+++ b/spring-batch-samples/src/test/java/org/springframework/batch/sample/RestartFunctionalTests.java
@@ -17,7 +17,6 @@
 package org.springframework.batch.sample;
 
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
 
 import javax.sql.DataSource;
 
@@ -62,10 +61,10 @@ public class RestartFunctionalTests extends AbstractBatchLauncherTests {
 	 * finish successfully, because it continues execution where the previous
 	 * run stopped (module throws exception after fixed number of processed
 	 * records).
-	 * @throws Exception the exception thrown
+	 * @throws Throwable 
 	 */
 	@Test
-	public void testRestart() throws Exception {
+	public void testRestart() throws Throwable {
 
 		int before = simpleJdbcTemplate.queryForInt("SELECT COUNT(*) FROM TRADE");
 
@@ -73,8 +72,10 @@ public class RestartFunctionalTests extends AbstractBatchLauncherTests {
 		assertEquals(BatchStatus.FAILED, jobExecution.getStatus());
 		
 		Throwable expected = jobExecution.getAllFailureExceptions().get(0);
-		assertTrue("Not planned exception: " + expected.getMessage(), expected.getMessage().toLowerCase().indexOf(
-					"planned") >= 0);
+		if(expected.getMessage().toLowerCase().indexOf(
+					"planned") < 0) {
+			throw expected;
+		}
 
 		int medium = simpleJdbcTemplate.queryForInt("SELECT COUNT(*) FROM TRADE");
 		// assert based on commit interval = 2
@@ -91,7 +92,7 @@ public class RestartFunctionalTests extends AbstractBatchLauncherTests {
 	// load the application context and launch the job
 	private JobExecution runJobForRestartTest() throws Exception {
 		return getLauncher().run(getJob(), new DefaultJobParametersConverter().getJobParameters(PropertiesConverter
-				.stringToProperties("parameter=true")));
+				.stringToProperties("run.id(long)=1,parameter=true,run.date=20070122,input.file=classpath:data/fixedLengthImportJob/input/20070122.teststream.ImportTradeDataStep.txt")));
 	}
 
 }