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 8af5203d2..033d1438e 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 @@ -52,8 +52,7 @@ import org.springframework.util.StringValueResolver; * @author Dave Syer * */ -public class PlaceholderTargetSource extends SimpleBeanTargetSource implements - InitializingBean { +public class PlaceholderTargetSource extends SimpleBeanTargetSource implements InitializingBean { /** * Key for placeholders to be replaced from the properties provided. @@ -70,8 +69,7 @@ public class PlaceholderTargetSource extends SimpleBeanTargetSource implements * Public setter for the context factory. Used to construct the context root * whenever placeholders are replaced in a bean definition. * - * @param contextFactory - * the {@link ContextFactory} + * @param contextFactory the {@link ContextFactory} */ public void setContextFactory(ContextFactory contextFactory) { this.contextFactory = contextFactory; @@ -104,56 +102,52 @@ public class PlaceholderTargetSource extends SimpleBeanTargetSource implements DefaultListableBeanFactory listableBeanFactory = (DefaultListableBeanFactory) getBeanFactory(); - final TypeConverter typeConverter = listableBeanFactory - .getTypeConverter(); + final TypeConverter typeConverter = listableBeanFactory.getTypeConverter(); - DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory( - listableBeanFactory); + DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory(listableBeanFactory); beanFactory.copyConfigurationFrom(listableBeanFactory); final TypeConverter contextTypeConverter = new TypeConverter() { @SuppressWarnings("unchecked") - public Object convertIfNecessary(Object value, Class requiredType, - MethodParameter methodParam) throws TypeMismatchException { + public Object convertIfNecessary(Object value, Class requiredType, MethodParameter methodParam) + throws TypeMismatchException { Object result = null; if (value instanceof String) { String key = (String) value; - if (key.startsWith(PLACEHOLDER_PREFIX) - && key.endsWith(PLACEHOLDER_SUFFIX)) { + if (key.startsWith(PLACEHOLDER_PREFIX) && key.endsWith(PLACEHOLDER_SUFFIX)) { key = extractKey(key); - result = convertFromContext(key, requiredType, - typeConverter); + 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); + property = convertToString(property, typeConverter); if (property != null) { value = property; } } + else { + throw new IllegalStateException("Cannot bind to placeholder: " + key); + } } } - } else if (requiredType.isAssignableFrom(value.getClass())) { + } + else if (requiredType.isAssignableFrom(value.getClass())) { result = value; - } else if (requiredType.isAssignableFrom(String.class)) { + } + else if (requiredType.isAssignableFrom(String.class)) { result = convertToString(value, typeConverter); if (result == null) { - logger - .debug("Falling back on toString for conversion of : [" - + value.getClass() + "]"); + logger.debug("Falling back on toString for conversion of : [" + value.getClass() + "]"); result = value.toString(); } } - return result != null ? result : typeConverter - .convertIfNecessary(value, requiredType, methodParam); + return result != null ? result : typeConverter.convertIfNecessary(value, requiredType, methodParam); } @SuppressWarnings("unchecked") - public Object convertIfNecessary(Object value, Class requiredType) - throws TypeMismatchException { + public Object convertIfNecessary(Object value, Class requiredType) throws TypeMismatchException { return convertIfNecessary(value, requiredType, null); } }; @@ -167,34 +161,27 @@ public class PlaceholderTargetSource extends SimpleBeanTargetSource implements * come back when getBean() is called later on */ String targetBeanName = getTargetBeanName(); - GenericBeanDefinition beanDefinition = new GenericBeanDefinition( - listableBeanFactory.getMergedBeanDefinition(targetBeanName)); + GenericBeanDefinition beanDefinition = new GenericBeanDefinition(listableBeanFactory + .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; - } - if (strVal.startsWith(PLACEHOLDER_PREFIX) - && strVal.endsWith(PLACEHOLDER_SUFFIX)) { - // If the whole value is a placeholder it might - // be - // possible to replace it all in one go as a - // String - // (e.g. if it's a ref=#{}) - StringBuilder result = new StringBuilder(strVal); - String key = extractKey(strVal); - replaceIfTypeMatches(result, 0, - strVal.length() - 1, key, String.class, - typeConverter); - return result.toString(); - } - return replacePlaceholders(strVal, - contextTypeConverter); - } - }) { + BeanDefinitionVisitor visitor = new BeanDefinitionVisitor(new StringValueResolver() { + public String resolveStringValue(String strVal) { + if (!strVal.contains(PLACEHOLDER_PREFIX)) { + return strVal; + } + if (strVal.startsWith(PLACEHOLDER_PREFIX) && strVal.endsWith(PLACEHOLDER_SUFFIX)) { + // If the whole value is a placeholder it might + // be possible to replace it all in one go as a + // String (e.g. if it's a ref=#{}) + StringBuilder result = new StringBuilder(strVal); + String key = extractKey(strVal); + replaceIfTypeMatches(result, 0, strVal.length() - 1, key, String.class, typeConverter); + return result.toString(); + } + return replacePlaceholders(strVal, contextTypeConverter); + } + }) { protected Object resolveValue(Object value) { if (value instanceof TypedStringValue) { TypedStringValue typedStringValue = (TypedStringValue) value; @@ -203,7 +190,8 @@ public class PlaceholderTargetSource extends SimpleBeanTargetSource implements String visitedString = resolveStringValue(stringValue); value = new TypedStringValue(visitedString); } - } else { + } + else { value = super.resolveValue(value); } return value; @@ -218,7 +206,8 @@ public class PlaceholderTargetSource extends SimpleBeanTargetSource implements putTargetInContext(target); return target; - } finally { + } + finally { beanFactory.removeBeanDefinition(beanName); beanFactory = null; // Anything else we can do to clean it up? @@ -251,13 +240,12 @@ public class PlaceholderTargetSource extends SimpleBeanTargetSource implements 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) { + result = (String) typeConverter.convertIfNecessary(value, String.class); + } + catch (TypeMismatchException e) { // ignore } - if (result == null - && typeConverter instanceof PropertyEditorRegistrySupport) { + if (result == null && typeConverter instanceof PropertyEditorRegistrySupport) { /* * PropertyEditorRegistrySupport is de rigeur with TypeConverter * instances used internally by Spring. If we have one of those then @@ -265,8 +253,7 @@ public class PlaceholderTargetSource extends SimpleBeanTargetSource implements * to. */ PropertyEditorRegistrySupport registry = (PropertyEditorRegistrySupport) typeConverter; - PropertyEditor editor = registry.findCustomEditor(value.getClass(), - null); + PropertyEditor editor = registry.findCustomEditor(value.getClass(), null); if (editor != null) { if (registry.isSharedEditor(editor)) { // Synchronized access to shared editor @@ -275,7 +262,8 @@ public class PlaceholderTargetSource extends SimpleBeanTargetSource implements editor.setValue(value); result = editor.getAsText(); } - } else { + } + else { editor.setValue(value); result = editor.getAsText(); } @@ -290,12 +278,10 @@ public class PlaceholderTargetSource extends SimpleBeanTargetSource implements * @param typeConverter * @return */ - private Object convertFromContext(String key, Class requiredType, - TypeConverter typeConverter) { + private Object convertFromContext(String key, Class requiredType, TypeConverter typeConverter) { Object result = null; Object property = getPropertyFromContext(key); - if (property == null - || requiredType.isAssignableFrom(property.getClass())) { + if (property == null || requiredType.isAssignableFrom(property.getClass())) { result = property; } return result; @@ -312,8 +298,7 @@ public class PlaceholderTargetSource extends SimpleBeanTargetSource implements 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()); + value = value.substring(0, value.length() - PLACEHOLDER_SUFFIX.length()); } return value; } @@ -332,28 +317,18 @@ public class PlaceholderTargetSource extends SimpleBeanTargetSource implements while (first >= 0) { - Assert - .state( - next > 0, - String - .format( - "Placeholder key incorrectly specified: use %skey%s (in %s)", - PLACEHOLDER_PREFIX, - PLACEHOLDER_SUFFIX, value)); + 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); + String key = result.substring(first + PLACEHOLDER_PREFIX.length(), next); - replaceIfTypeMatches(result, first, next, key, String.class, - typeConverter); - replaceIfTypeMatches(result, first, next, key, Long.class, - typeConverter); - replaceIfTypeMatches(result, first, next, key, Integer.class, - typeConverter); - // Spring cannot convert from String to Date, so there is an error - // here. - replaceIfTypeMatches(result, first, next, key, Date.class, - typeConverter); + 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) { + throw new IllegalStateException("Cannot bind to placeholder: "+key); + } first = result.indexOf(PLACEHOLDER_PREFIX, first + 1); next = result.indexOf(PLACEHOLDER_SUFFIX, first + 1); @@ -364,14 +339,14 @@ public class PlaceholderTargetSource extends SimpleBeanTargetSource implements } - private void replaceIfTypeMatches(StringBuilder result, int first, - int next, String key, Class requiredType, + 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)); + 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/PlaceholderTargetSourceErrorTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/scope/util/PlaceholderTargetSourceErrorTests.java index 5781f0bf5..f0c1abb11 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 @@ -72,12 +72,38 @@ public class PlaceholderTargetSourceErrorTests extends ContextFactorySupport { assertEquals("bar-bar", target.getName()); } + @Test + public void testPartialReplaceMissingProperty() throws Exception { + try { + Node target = (Node) createValue("name", "#{garbage}-bar").getTarget(); + assertEquals("bar", target.getName()); + fail("Expected IllegalStateException"); + } + catch (Exception e) { + String message = e.getMessage(); + assertTrue("Wrong message: " + message, message.toLowerCase().contains("cannot bind")); + } + } + @Test public void testFullReplaceSunnyDay() throws Exception { Node target = (Node) createValue("name", "#{foo}").getTarget(); assertEquals("bar", target.getName()); } + @Test + public void testFullReplaceMissingProperty() throws Exception { + try { + Node target = (Node) createValue("name", "#{garbage}").getTarget(); + assertEquals("bar", target.getName()); + fail("Expected IllegalStateException"); + } + catch (Exception e) { + String message = e.getMessage(); + assertTrue("Wrong message: " + message, message.toLowerCase().contains("cannot bind")); + } + } + @Test public void testPartialReplaceIntegerToString() throws Exception { Node target = (Node) createValue("name", "foo-#{integer}").getTarget(); 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 6791753df..05825d20b 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 @@ -47,10 +47,6 @@ public class PlaceholderTargetSourceTests extends ContextFactorySupport { @Qualifier("withDate") private PlaceholderTargetSource withDate; - @Autowired - @Qualifier("withNull") - private PlaceholderTargetSource withNull; - @Autowired @Qualifier("compound") private PlaceholderTargetSource compound; @@ -172,13 +168,6 @@ public class PlaceholderTargetSourceTests extends ContextFactorySupport { assertEquals(1L, target.getDate().getTime()); } - @Test - public void testGetNull() { - Node target = (Node) withNull.getTarget(); - // Remains unconverted because null is explicitly excluded - assertEquals("bar-#{garbage}", target.getName()); - } - public static interface Node { String getName(); 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 e5a5691bf..c50e4bbd3 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 @@ -129,18 +129,6 @@ - - - - - - - - -