From 7c487a7abc86276df59212a6d4fd2fd207047124 Mon Sep 17 00:00:00 2001 From: dsyer Date: Fri, 14 Aug 2009 09:35:50 +0000 Subject: [PATCH] RESOLVED - issue BATCH-1370: Bind to non-scalar map entry values in step scope Also fixed: no need to use custom PropertyEditor to bind to Dates from step context --- spring-batch-core/.springBeans | 4 +- .../scope/util/PlaceholderTargetSource.java | 204 ++++++++++++------ ...ceholderTargetSourceCustomEditorTests.java | 61 ++++++ .../PlaceholderTargetSourceErrorTests.java | 18 +- .../util/PlaceholderTargetSourceTests.java | 46 +++- ...rTargetSourceCustomEditorTests-context.xml | 48 +++++ .../PlaceholderTargetSourceTests-context.xml | 53 +++-- 7 files changed, 330 insertions(+), 104 deletions(-) create mode 100644 spring-batch-core/src/test/java/org/springframework/batch/core/scope/util/PlaceholderTargetSourceCustomEditorTests.java create mode 100644 spring-batch-core/src/test/resources/org/springframework/batch/core/scope/util/PlaceholderTargetSourceCustomEditorTests-context.xml diff --git a/spring-batch-core/.springBeans b/spring-batch-core/.springBeans index 8777342d8..e3950f15b 100644 --- a/spring-batch-core/.springBeans +++ b/spring-batch-core/.springBeans @@ -1,7 +1,7 @@ 1 - + @@ -61,6 +61,8 @@ src/test/resources/org/springframework/batch/core/configuration/xml/JobRegistryJobParserTests-context.xml src/test/resources/org/springframework/batch/core/launch/JobLauncherIntegrationTests-context.xml src/test/resources/org/springframework/batch/core/configuration/xml/StopCustomStatusJobParserTests-context.xml + src/test/resources/org/springframework/batch/core/scope/util/PlaceholderTargetSourceCustomEditorTests-context.xml + 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 451695fac..9d0cc2403 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 @@ -39,14 +39,14 @@ import org.springframework.util.StringValueResolver; /** * A {@link TargetSource} that lazily initializes its target, replacing bean * definition properties dynamically if they are marked as placeholders. String - * values with embedded #{key} patterns will be replaced with the + * values with embedded %{key} patterns will be replaced with the * corresponding value from the injected context (which must also be a String). * This includes dynamically locating a bean reference (e.g. - * ref="#{foo}"), and partial replacement of patterns (e.g. - * value="#{foo}-bar-#{spam}"). These replacements work for context + * ref="%{foo}"), and partial replacement of patterns (e.g. + * value="%{foo}-bar-%{spam}"). These replacements work for context * values that are primitive (String, Long, Integer). You can also replace * non-primitive values directly by making the whole bean property value into a - * placeholder (e.g. value="#{foo}" where foo is a + * placeholder (e.g. value="%{foo}" where foo is a * property in the context). * * @author Dave Syer @@ -116,7 +116,7 @@ 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, typeConverter); + result = convertFromContext(key, requiredType); if (result == null) { Object property = getPropertyFromContext(key); // Give the normal type converter a chance by @@ -126,11 +126,15 @@ public class PlaceholderTargetSource extends SimpleBeanTargetSource implements I if (property != null) { value = property; } + logger.debug(String.format("Bound %%{%s} to String value [%s]", key, result)); } else { throw new IllegalStateException("Cannot bind to placeholder: " + key); } } + else { + logger.debug(String.format("Bound %%{%s} to [%s]", key, result)); + } } } else if (requiredType.isAssignableFrom(value.getClass())) { @@ -165,30 +169,7 @@ public class PlaceholderTargetSource extends SimpleBeanTargetSource implements I .getMergedBeanDefinition(targetBeanName)); logger.debug("Rehydrating scoped target: [" + targetBeanName + "]"); - BeanDefinitionVisitor visitor = new BeanDefinitionVisitor(new StringValueResolver() { - public String resolveStringValue(String strVal) { - if (!strVal.contains(PLACEHOLDER_PREFIX)) { - return strVal; - } - return replacePlaceholders(strVal, contextTypeConverter); - } - }) { - protected Object resolveValue(Object value) { - if (value instanceof TypedStringValue) { - TypedStringValue typedStringValue = (TypedStringValue) value; - String stringValue = typedStringValue.getValue(); - if (stringValue != null) { - String visitedString = resolveStringValue(stringValue); - value = new TypedStringValue(visitedString); - } - } - else { - value = super.resolveValue(value); - } - return value; - } - - }; + BeanDefinitionVisitor visitor = new PlaceholderBeanDefinitionVisitor(contextTypeConverter); beanFactory.registerBeanDefinition(beanName, beanDefinition); // Make the replacements before the target is hydrated @@ -266,10 +247,9 @@ public class PlaceholderTargetSource extends SimpleBeanTargetSource implements I /** * @param value * @param requiredType - * @param typeConverter * @return */ - private Object convertFromContext(String key, Class requiredType, TypeConverter typeConverter) { + private Object convertFromContext(String key, Class requiredType) { Object result = null; Object property = getPropertyFromContext(key); if (property == null || requiredType.isAssignableFrom(property.getClass())) { @@ -287,57 +267,141 @@ public class PlaceholderTargetSource extends SimpleBeanTargetSource implements I } private String extractKey(String value) { - if (value.startsWith(PLACEHOLDER_PREFIX)) { - value = value.substring(PLACEHOLDER_PREFIX.length()); - value = value.substring(0, value.length() - PLACEHOLDER_SUFFIX.length()); - } - return value; + return value.substring(value.indexOf(PLACEHOLDER_PREFIX) + PLACEHOLDER_PREFIX.length(), value + .indexOf(PLACEHOLDER_SUFFIX)); } /** - * @param typeConverter - * @param strVal - * @return + * Determine whether the input is a whole key in the form + * %{...}, i.e. starting with the correct prefix, ending with + * the correct suffix and containing only one of each. + * + * @param value a String with placeholder patterns + * @return true if the value is a key */ - private String replacePlaceholders(String value, TypeConverter typeConverter) { + private boolean isKey(String value) { + return value.indexOf(PLACEHOLDER_PREFIX) == value.lastIndexOf(PLACEHOLDER_PREFIX) + && value.startsWith(PLACEHOLDER_PREFIX) && value.endsWith(PLACEHOLDER_SUFFIX); + } - StringBuilder result = new StringBuilder(value); - - int first = result.indexOf(PLACEHOLDER_PREFIX); - int next = result.indexOf(PLACEHOLDER_SUFFIX, first + 1); - - while (first >= 0) { - - Assert.state(next > 0, String.format("Placeholder key incorrectly specified: use %skey%s (in %s)", - PLACEHOLDER_PREFIX, PLACEHOLDER_SUFFIX, value)); - - String key = result.substring(first + PLACEHOLDER_PREFIX.length(), next); - - boolean replaced = replaceIfTypeMatches(result, first, next, key, String.class, typeConverter); - replaced |= replaceIfTypeMatches(result, first, next, key, Long.class, typeConverter); - replaced |= replaceIfTypeMatches(result, first, next, key, Integer.class, typeConverter); - replaced |= replaceIfTypeMatches(result, first, next, key, Date.class, typeConverter); - if (!replaced) { - logger.debug("Cannot bind to placeholder: " + key); - } - first = result.indexOf(PLACEHOLDER_PREFIX, first + 1); - next = result.indexOf(PLACEHOLDER_SUFFIX, first + 1); + /** + * A {@link BeanDefinitionVisitor} that will replace embedded placeholders + * with values from the provided context. + * + * @author Dave Syer + * + */ + private final class PlaceholderBeanDefinitionVisitor extends BeanDefinitionVisitor { + public PlaceholderBeanDefinitionVisitor(final TypeConverter typeConverter) { + super(new PlaceholderStringValueResolver(typeConverter)); } - logger.debug(String.format("Replaced [%s] with [%s]", value, result)); - return result.toString(); + protected Object resolveValue(Object value) { + + if (value instanceof TypedStringValue) { + + TypedStringValue typedStringValue = (TypedStringValue) value; + String stringValue = typedStringValue.getValue(); + if (stringValue != null) { + + // If the value is a whole key, try to simply replace it from context. + if (isKey(stringValue)) { + Object result = getPropertyFromContext(extractKey(stringValue)); + if (result != null) { + value = result; + } + } + else { + // Otherwise it might contain embedded keys so we try to replace those + String visitedString = resolveStringValue(stringValue); + typedStringValue.setValue(visitedString); + } + } + + } + else { + + value = super.resolveValue(value); + + } + + return value; + + } } - private boolean replaceIfTypeMatches(StringBuilder result, int first, int next, String key, Class requiredType, - TypeConverter typeConverter) { - Object property = convertFromContext(key, requiredType, typeConverter); - if (property != null) { - result.replace(first, next + 1, (String) typeConverter.convertIfNecessary(property, String.class)); - return true; + private final class PlaceholderStringValueResolver implements StringValueResolver { + + private final TypeConverter typeConverter; + + private PlaceholderStringValueResolver(TypeConverter typeConverter) { + this.typeConverter = typeConverter; } - return false; + + public String resolveStringValue(String strVal) { + if (!strVal.contains(PLACEHOLDER_PREFIX)) { + return strVal; + } + return replacePlaceholders(strVal, typeConverter); + } + + /** + * Convenience method to replace all the placeholders in the input. + * + * @param typeConverter a {@link TypeConverter} that can be used to + * convert placeholder keys to context values + * @param value the value to replace placeholders in + * @return the input with placeholders replaced + */ + private String replacePlaceholders(String value, TypeConverter typeConverter) { + + StringBuilder result = new StringBuilder(value); + + int first = result.indexOf(PLACEHOLDER_PREFIX); + int next = result.indexOf(PLACEHOLDER_SUFFIX, first + 1); + + while (first >= 0) { + + Assert.state(next > 0, String.format("Placeholder key incorrectly specified: use %skey%s (in %s)", + PLACEHOLDER_PREFIX, PLACEHOLDER_SUFFIX, value)); + + String key = result.substring(first + PLACEHOLDER_PREFIX.length(), next); + + boolean replaced = replaceIfTypeMatches(result, first, next, key, String.class, typeConverter); + replaced |= replaceIfTypeMatches(result, first, next, key, Long.class, typeConverter); + replaced |= replaceIfTypeMatches(result, first, next, key, Integer.class, typeConverter); + replaced |= replaceIfTypeMatches(result, first, next, key, Date.class, typeConverter); + if (!replaced) { + if (!value.startsWith(PLACEHOLDER_PREFIX) || !value.endsWith(PLACEHOLDER_SUFFIX)) { + throw new IllegalStateException(String.format("Cannot bind to partial key %%{%s} in %s", key, + value)); + } + logger.debug(String.format("Deferring binding of placeholder: %%{%s}", key)); + } + else { + logger.debug(String.format("Bound %%{%s} to obtain [%s]", key, result)); + } + first = result.indexOf(PLACEHOLDER_PREFIX, first + 1); + next = result.indexOf(PLACEHOLDER_SUFFIX, first + 1); + + } + + return result.toString(); + + } + + private boolean replaceIfTypeMatches(StringBuilder result, int first, int next, String key, + Class requiredType, TypeConverter typeConverter) { + Object property = convertFromContext(key, requiredType); + if (property != null) { + result.replace(first, next + 1, (String) typeConverter.convertIfNecessary(property, String.class)); + return true; + } + return false; + } + } } diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/scope/util/PlaceholderTargetSourceCustomEditorTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/scope/util/PlaceholderTargetSourceCustomEditorTests.java new file mode 100644 index 000000000..65c350ca5 --- /dev/null +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/scope/util/PlaceholderTargetSourceCustomEditorTests.java @@ -0,0 +1,61 @@ +package org.springframework.batch.core.scope.util; + +import static org.junit.Assert.assertEquals; + +import java.text.SimpleDateFormat; +import java.util.Date; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; + +@ContextConfiguration +@RunWith(SpringJUnit4ClassRunner.class) +public class PlaceholderTargetSourceCustomEditorTests extends ContextFactorySupport { + + @Autowired + @Qualifier("withEmbeddedDate") + private PlaceholderTargetSource withEmbeddedDate; + + private Date date = new Date(1L); + + public Object getContext() { + return this; + } + + public Date getDate() { + return date; + } + + @Test + public void testGetEmbeddedDate() { + Node target = (Node) withEmbeddedDate.getTarget(); + String date = new SimpleDateFormat("yyyy/MM/dd").format(new Date(1L)); + assertEquals("bar-"+date, target.getName()); + } + + public static interface Node { + String getName(); + } + + public static class Foo implements Node { + + private String name; + + public Foo() { + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + } + +} 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 index 619e8b3bd..b5864c963 100644 --- 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 @@ -10,6 +10,7 @@ import java.util.Map; import org.apache.commons.io.IOUtils; import org.junit.Test; +import org.springframework.beans.factory.BeanCreationException; import org.springframework.beans.factory.xml.XmlBeanDefinitionReader; import org.springframework.context.support.GenericApplicationContext; import org.springframework.core.io.ByteArrayResource; @@ -74,8 +75,15 @@ public class PlaceholderTargetSourceErrorTests extends ContextFactorySupport { @Test public void testPartialReplaceMissingProperty() throws Exception { - Node target = (Node) createValue("name", "%{garbage}-bar").getTarget(); - assertEquals("%{garbage}-bar", target.getName()); + try { + Node target = (Node) createValue("name", "%{garbage}-bar").getTarget(); + assertEquals("%{garbage}-bar", target.getName()); + fail("Expected IllegalStateException"); + } + catch (IllegalStateException e) { + String message = e.getMessage(); + assertTrue("Wrong message: " + message, message.toLowerCase().contains("cannot bind")); + } } @Test @@ -91,7 +99,7 @@ public class PlaceholderTargetSourceErrorTests extends ContextFactorySupport { assertEquals("bar", target.getName()); fail("Expected IllegalStateException"); } - catch (Exception e) { + catch (BeanCreationException e) { String message = e.getMessage(); assertTrue("Wrong message: " + message, message.toLowerCase().contains("cannot bind")); } @@ -99,8 +107,8 @@ public class PlaceholderTargetSourceErrorTests extends ContextFactorySupport { @Test public void testPartialReplaceIntegerToString() throws Exception { - Node target = (Node) createValue("name", "foo-%{integer}").getTarget(); - assertEquals("foo-4321", target.getName()); + Node target = (Node) createValue("name", "foo-%{integer}").getTarget(); + assertEquals("foo-4321", target.getName()); } @Test diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/scope/util/PlaceholderTargetSourceTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/scope/util/PlaceholderTargetSourceTests.java index b4e253440..1bf7ae3c7 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/scope/util/PlaceholderTargetSourceTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/scope/util/PlaceholderTargetSourceTests.java @@ -3,7 +3,7 @@ package org.springframework.batch.core.scope.util; import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; -import java.text.SimpleDateFormat; +import java.util.Arrays; import java.util.Collections; import java.util.Date; import java.util.List; @@ -40,6 +40,14 @@ public class PlaceholderTargetSourceTests extends ContextFactorySupport { @Qualifier("withList") private PlaceholderTargetSource withList; + @Autowired + @Qualifier("withLiteralList") + private PlaceholderTargetSource withLiteralList; + + @Autowired + @Qualifier("withMap") + private PlaceholderTargetSource withMap; + @Autowired @Qualifier("withMultiple") private PlaceholderTargetSource withMultiple; @@ -84,6 +92,10 @@ public class PlaceholderTargetSourceTests extends ContextFactorySupport { return map; } + public List getList() { + return Arrays.asList("bar", "spam"); + } + public Node getParent() { return new Foo("spam"); } @@ -165,6 +177,20 @@ public class PlaceholderTargetSourceTests extends ContextFactorySupport { assertEquals("[bar, foo-4321, bar-4321]", target.getList().toString()); } + @Test + public void testGetLiteralList() { + Node target = (Node) withLiteralList.getTarget(); + assertEquals(2, target.getList().size()); + assertEquals("[bar, spam]", target.getList().toString()); + } + + @Test + public void testGetMap() { + Node target = (Node) withMap.getTarget(); + assertEquals(3, target.getMap().size()); + assertEquals("{foo=bar, bar=foo-4321, spam=[bar, spam]}", target.getMap().toString()); + } + @Test public void testGetMultiple() { Node target = (Node) withMultiple.getTarget(); @@ -180,16 +206,14 @@ public class PlaceholderTargetSourceTests extends ContextFactorySupport { @Test public void testGetEmbeddedDate() { Node target = (Node) withEmbeddedDate.getTarget(); - String date = new SimpleDateFormat("yyyy/MM/dd").format(new Date(1L)); + String date = new Date(1L).toString(); assertEquals("bar-"+date, target.getName()); } @Test public void testGetDate() { Node target = (Node) withDate.getTarget(); - SimpleDateFormat sdf = new SimpleDateFormat("yyyy/MM/dd"); - String date = sdf.format(new Date(1L)); - assertEquals(date, sdf.format(target.getDate())); + assertEquals(new Date(1L), target.getDate()); } public static interface Node { @@ -200,6 +224,8 @@ public class PlaceholderTargetSourceTests extends ContextFactorySupport { Node getParent(); List getList(); + + Map getMap(); } public static class Foo implements Node { @@ -212,6 +238,8 @@ public class PlaceholderTargetSourceTests extends ContextFactorySupport { private List list; + private Map map; + public Foo() { } @@ -251,6 +279,14 @@ public class PlaceholderTargetSourceTests extends ContextFactorySupport { return list; } + public Map getMap() { + return map; + } + + public void setMap(Map map) { + this.map = map; + } + } } diff --git a/spring-batch-core/src/test/resources/org/springframework/batch/core/scope/util/PlaceholderTargetSourceCustomEditorTests-context.xml b/spring-batch-core/src/test/resources/org/springframework/batch/core/scope/util/PlaceholderTargetSourceCustomEditorTests-context.xml new file mode 100644 index 000000000..d32ab9bf2 --- /dev/null +++ b/spring-batch-core/src/test/resources/org/springframework/batch/core/scope/util/PlaceholderTargetSourceCustomEditorTests-context.xml @@ -0,0 +1,48 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ 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 565df2acb..fc2094af5 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 @@ -58,6 +58,18 @@ + + + + + + + + + + @@ -135,6 +147,24 @@ + + + + + + + + + + + + + + @@ -159,27 +189,4 @@ - - - - - - - - - - - - - - - - - - - - - \ No newline at end of file