From e692072862b489537a6e8b94a89b44352a3f437c Mon Sep 17 00:00:00 2001 From: robokaso Date: Wed, 7 Jan 2009 10:36:36 +0000 Subject: [PATCH] RESOLVED - BATCH-988: MethodInvokingTaskletAdapter (via AbstractMethodInvokingDelegator) only allows specification of targetObject's declared methods use getMethods() instead of getDeclaredMethods for validating target method exists allow null argument values --- .../AbstractMethodInvokingDelegator.java | 87 +++++++++++-------- .../item/adapter/AbstractDelegatorTests.java | 35 +++++--- 2 files changed, 75 insertions(+), 47 deletions(-) diff --git a/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/adapter/AbstractMethodInvokingDelegator.java b/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/adapter/AbstractMethodInvokingDelegator.java index 417e3bd60..a1cfcb958 100644 --- a/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/adapter/AbstractMethodInvokingDelegator.java +++ b/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/adapter/AbstractMethodInvokingDelegator.java @@ -24,56 +24,59 @@ import org.springframework.util.Assert; import org.springframework.util.MethodInvoker; /** - * Superclass for delegating classes which dynamically call a - * custom method of injected object. - * Provides convenient API for dynamic method invocation shielding - * subclasses from low-level details and exception handling. + * Superclass for delegating classes which dynamically call a custom method of + * injected object. Provides convenient API for dynamic method invocation + * shielding subclasses from low-level details and exception handling. * * @author Robert Kasanicky */ public abstract class AbstractMethodInvokingDelegator implements InitializingBean { - + private Object targetObject; - + private String targetMethod; - + private Object[] arguments; /** - * Invoker the target method with no arguments. + * Invoker the target method with arguments set by + * {@link #setArguments(Object[])}. * @return object returned by invoked method - * @throws DynamicMethodInvocationException if the {@link MethodInvoker} used throws exception + * @throws DynamicMethodInvocationException if the {@link MethodInvoker} + * used throws exception */ protected T invokeDelegateMethod() { MethodInvoker invoker = createMethodInvoker(targetObject, targetMethod); invoker.setArguments(arguments); return doInvoke(invoker); } - + /** * Invokes the target method with given argument. * @param object argument for the target method * @return object returned by target method - * @throws DynamicMethodInvocationException if the {@link MethodInvoker} used throws exception + * @throws DynamicMethodInvocationException if the {@link MethodInvoker} + * used throws exception */ protected T invokeDelegateMethodWithArgument(Object object) { MethodInvoker invoker = createMethodInvoker(targetObject, targetMethod); - invoker.setArguments(new Object[]{object}); + invoker.setArguments(new Object[] { object }); return doInvoke(invoker); } - + /** * Invokes the target method with given arguments. * @param args arguments for the invoked method * @return object returned by invoked method - * @throws DynamicMethodInvocationException if the {@link MethodInvoker} used throws exception + * @throws DynamicMethodInvocationException if the {@link MethodInvoker} + * used throws exception */ protected T invokeDelegateMethodWithArguments(Object[] args) { MethodInvoker invoker = createMethodInvoker(targetObject, targetMethod); invoker.setArguments(args); return doInvoke(invoker); } - + /** * Create a new configured instance of {@link MethodInvoker}. */ @@ -84,7 +87,7 @@ public abstract class AbstractMethodInvokingDelegator implements Initializing invoker.setArguments(arguments); return invoker; } - + /** * Prepare and invoke the invoker, rethrow checked exceptions as unchecked. * @param invoker configured invoker @@ -101,7 +104,7 @@ public abstract class AbstractMethodInvokingDelegator implements Initializing catch (NoSuchMethodException e) { throw new DynamicMethodInvocationException(e); } - + try { return (T) invoker.invoke(); } @@ -110,64 +113,78 @@ public abstract class AbstractMethodInvokingDelegator implements Initializing } catch (IllegalAccessException e) { throw new DynamicMethodInvocationException(e); - } + } } public void afterPropertiesSet() throws Exception { Assert.notNull(targetObject); Assert.hasLength(targetMethod); - Assert.state(targetClassDeclaresTargetMethod(), - "target class must declare a method with name matching the target method"); + Assert.state(targetClassDeclaresTargetMethod(), + "target class must declare a method with matching name and parameter types"); } - + /** - * @return true if target class declares a method matching target method name - * with given number of arguments of appropriate type. + * @return true if target class declares a method matching target method + * name with given number of arguments of appropriate type. */ private boolean targetClassDeclaresTargetMethod() { MethodInvoker invoker = createMethodInvoker(targetObject, targetMethod); - Method[] methods = invoker.getTargetClass().getDeclaredMethods(); + Method[] methods = invoker.getTargetClass().getMethods(); String targetMethodName = invoker.getTargetMethod(); - for (int i=0; i < methods.length; i++) { + for (int i = 0; i < methods.length; i++) { if (methods[i].getName().equals(targetMethodName)) { Class[] params = methods[i].getParameterTypes(); if (arguments == null) { + // don't check signature, assume arguments will be supplied + // correctly at runtime return true; - } else if (arguments.length == params.length) { + } + if (arguments.length == params.length) { boolean argumentsMatchParameters = true; for (int j = 0; j < params.length; j++) { + if (arguments[j] == null) { + continue; + } if (!(params[j].isAssignableFrom(arguments[j].getClass()))) { argumentsMatchParameters = false; } } - if (argumentsMatchParameters) return true; + if (argumentsMatchParameters) + return true; } } } - + return false; } /** - * @param targetObject the delegate - bean id can be used to set this value in Spring configuration + * @param targetObject the delegate - bean id can be used to set this value + * in Spring configuration */ public void setTargetObject(Object targetObject) { this.targetObject = targetObject; } /** - * @param targetMethod name of the method to be invoked on {@link #setTargetObject(Object)}. + * @param targetMethod name of the method to be invoked on + * {@link #setTargetObject(Object)}. */ public void setTargetMethod(String targetMethod) { this.targetMethod = targetMethod; } - + /** - * @param arguments arguments values for the {{@link #setTargetMethod(String)}. - * These are not expected to change during the lifetime of the delegator - * and will be used only when the subclass tries to invoke the target method - * without providing explicit argument values. + * @param arguments arguments values for the { + * {@link #setTargetMethod(String)}. These will be used only when the + * subclass tries to invoke the target method without providing explicit + * argument values. + * + * If arguments are set to not-null value {@link #afterPropertiesSet()} will + * check the values are compatible with target method's signature. In case + * arguments are null (not set) method signature will not be checked and it + * is assumed correct values will be supplied at runtime. */ public void setArguments(Object[] arguments) { this.arguments = arguments; diff --git a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/adapter/AbstractDelegatorTests.java b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/adapter/AbstractDelegatorTests.java index b0c08efa5..6ba8f6527 100644 --- a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/adapter/AbstractDelegatorTests.java +++ b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/adapter/AbstractDelegatorTests.java @@ -10,7 +10,7 @@ import org.springframework.util.Assert; /** * Tests for {@link AbstractMethodInvokingDelegator} - * + * * @author Robert Kasanicky */ public class AbstractDelegatorTests extends TestCase { @@ -58,7 +58,19 @@ public class AbstractDelegatorTests extends TestCase { delegator.invokeDelegateMethod(); assertEquals(NEW_FOO_NAME, foo.getName()); } - + + /** + * Null argument value doesn't cause trouble when validating method + * signature. + */ + public void testDelegationWithCheckedNullArgument() throws Exception { + delegator.setTargetMethod("setName"); + delegator.setArguments(new Object[] { null }); + delegator.afterPropertiesSet(); + delegator.invokeDelegateMethod(); + assertNull(foo.getName()); + } + /** * Regular use - calling methods directly and via delegator leads to same * results @@ -68,11 +80,11 @@ public class AbstractDelegatorTests extends TestCase { delegator.setTargetObject(fooService); delegator.setTargetMethod("processNameValuePair"); delegator.afterPropertiesSet(); - + final String FOO_NAME = "fooName"; final int FOO_VALUE = 12345; - delegator.invokeDelegateMethodWithArguments(new Object[]{FOO_NAME, Integer.valueOf(FOO_VALUE)}); + delegator.invokeDelegateMethodWithArguments(new Object[] { FOO_NAME, Integer.valueOf(FOO_VALUE) }); Foo foo = (Foo) fooService.getProcessedFooNameValuePairs().get(0); assertEquals(FOO_NAME, foo.getName()); assertEquals(FOO_VALUE, foo.getValue()); @@ -134,22 +146,22 @@ public class AbstractDelegatorTests extends TestCase { try { // single argument expected but two provided - delegator.invokeDelegateMethodWithArguments(new Object[]{"name", "anotherName"}); + delegator.invokeDelegateMethodWithArguments(new Object[] { "name", "anotherName" }); fail(); } catch (DynamicMethodInvocationException e) { // expected } } - + /** * Exception scenario - incorrect static arguments set. */ public void testIncorrectNumberOfStaticArguments() throws Exception { delegator.setTargetMethod("setName"); - + // incorrect argument count - delegator.setArguments(new Object[]{"first", "second"}); + delegator.setArguments(new Object[] { "first", "second" }); try { delegator.afterPropertiesSet(); fail(); @@ -157,9 +169,9 @@ public class AbstractDelegatorTests extends TestCase { catch (IllegalStateException e) { // expected } - + // correct argument count, but invalid argument type - delegator.setArguments(new Object[]{new Object()}); + delegator.setArguments(new Object[] { new Object() }); try { delegator.afterPropertiesSet(); fail(); @@ -168,6 +180,5 @@ public class AbstractDelegatorTests extends TestCase { // expected } } - - + }