From db14e83fdc724dc38ebf95cb6c2fdb9e5afe5a97 Mon Sep 17 00:00:00 2001 From: dsyer Date: Tue, 12 Jan 2010 17:02:31 +0000 Subject: [PATCH] BATCH-1479: fix test and tidy up signature test --- .../item/adapter/HippyMethodInvoker.java | 29 +++- .../item/adapter/AbstractDelegatorTests.java | 126 ++++++++++++++---- 2 files changed, 122 insertions(+), 33 deletions(-) diff --git a/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/adapter/HippyMethodInvoker.java b/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/adapter/HippyMethodInvoker.java index a822849ef..3e6fb633b 100644 --- a/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/adapter/HippyMethodInvoker.java +++ b/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/adapter/HippyMethodInvoker.java @@ -36,38 +36,55 @@ public class HippyMethodInvoker extends MethodInvoker { protected Method findMatchingMethod() { String targetMethod = getTargetMethod(); Object[] arguments = getArguments(); - Object[] transformedArguments = arguments; int argCount = arguments.length; Method[] candidates = ReflectionUtils.getAllDeclaredMethods(getTargetClass()); int minTypeDiffWeight = Integer.MAX_VALUE; Method matchingMethod = null; + Object[] transformedArguments = null; + int transformedArgumentCount = 0; + for (int i = 0; i < candidates.length; i++) { Method candidate = candidates[i]; if (candidate.getName().equals(targetMethod)) { Class[] paramTypes = candidate.getParameterTypes(); - transformedArguments = new Object[paramTypes.length]; + Object[] candidateArguments = new Object[paramTypes.length]; + int assignedParameterCount = 0; for (int j = 0; j < arguments.length; j++) { for (int k = 0; k < paramTypes.length; k++) { - if (ClassUtils.isAssignableValue(paramTypes[k], arguments[j])) { - transformedArguments[k] = arguments[j]; + // Pick the first assignable of the right type that + // matches this slot... + if (ClassUtils.isAssignableValue(paramTypes[k], arguments[j]) + && candidateArguments[k] == null) { + candidateArguments[k] = arguments[j]; + assignedParameterCount++; } } } if (paramTypes.length <= argCount) { - int typeDiffWeight = getTypeDifferenceWeight(paramTypes, transformedArguments); + int typeDiffWeight = getTypeDifferenceWeight(paramTypes, candidateArguments); if (typeDiffWeight < minTypeDiffWeight) { minTypeDiffWeight = typeDiffWeight; matchingMethod = candidate; + transformedArguments = candidateArguments; + transformedArgumentCount = assignedParameterCount; } } } } + + if (transformedArguments==null) { + throw new IllegalArgumentException("No matching arguments found for method: "+targetMethod); + } + + if (transformedArgumentCount < transformedArguments.length) { + throw new IllegalArgumentException("Only " + transformedArgumentCount + " out of " + + transformedArguments.length + " arguments could be assigned."); + } setArguments(transformedArguments); return matchingMethod; } - } 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 1b190c035..3951989cf 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 @@ -1,12 +1,15 @@ package org.springframework.batch.item.adapter; -import junit.framework.TestCase; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.fail; -import org.springframework.batch.item.adapter.AbstractMethodInvokingDelegator; -import org.springframework.batch.item.adapter.DynamicMethodInvocationException; +import java.util.ArrayList; +import java.util.List; + +import org.junit.Before; +import org.junit.Test; import org.springframework.batch.item.adapter.AbstractMethodInvokingDelegator.InvocationTargetThrowableWrapper; -import org.springframework.batch.item.sample.Foo; -import org.springframework.batch.item.sample.FooService; import org.springframework.util.Assert; /** @@ -14,16 +17,17 @@ import org.springframework.util.Assert; * * @author Robert Kasanicky */ -public class AbstractDelegatorTests extends TestCase { +public class AbstractDelegatorTests { private static class ConcreteDelegator extends AbstractMethodInvokingDelegator { } private AbstractMethodInvokingDelegator delegator = new ConcreteDelegator(); - private Foo foo = new Foo(0, "foo", 1); + private Foo foo = new Foo("foo", 1); - protected void setUp() throws Exception { + @Before + public void setUp() throws Exception { delegator.setTargetObject(foo); delegator.setArguments(null); } @@ -32,6 +36,7 @@ public class AbstractDelegatorTests extends TestCase { * Regular use - calling methods directly and via delegator leads to same * results */ + @Test public void testDelegation() throws Exception { delegator.setTargetMethod("getName"); delegator.afterPropertiesSet(); @@ -43,6 +48,7 @@ public class AbstractDelegatorTests extends TestCase { * Regular use - calling methods directly and via delegator leads to same * results */ + @Test public void testDelegationWithArgument() throws Exception { delegator.setTargetMethod("setName"); final String NEW_FOO_NAME = "newFooName"; @@ -64,6 +70,7 @@ public class AbstractDelegatorTests extends TestCase { * Null argument value doesn't cause trouble when validating method * signature. */ + @Test public void testDelegationWithCheckedNullArgument() throws Exception { delegator.setTargetMethod("setName"); delegator.setArguments(new Object[] { null }); @@ -76,6 +83,7 @@ public class AbstractDelegatorTests extends TestCase { * Regular use - calling methods directly and via delegator leads to same * results */ + @Test public void testDelegationWithMultipleArguments() throws Exception { FooService fooService = new FooService(); delegator.setTargetObject(fooService); @@ -85,7 +93,7 @@ public class AbstractDelegatorTests extends TestCase { 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, FOO_VALUE }); Foo foo = (Foo) fooService.getProcessedFooNameValuePairs().get(0); assertEquals(FOO_NAME, foo.getName()); assertEquals(FOO_VALUE, foo.getValue()); @@ -94,12 +102,13 @@ public class AbstractDelegatorTests extends TestCase { /** * Exception scenario - target method is not declared by target object. */ + @Test public void testInvalidMethodName() throws Exception { delegator.setTargetMethod("not-existing-method-name"); try { delegator.afterPropertiesSet(); - fail(); + fail("Expected IllegalStateException"); } catch (IllegalStateException e) { // expected @@ -107,9 +116,9 @@ public class AbstractDelegatorTests extends TestCase { try { delegator.invokeDelegateMethod(); - fail(); + fail("Expected IllegalArgumentException"); } - catch (DynamicMethodInvocationException e) { + catch (IllegalArgumentException e) { // expected } } @@ -117,14 +126,15 @@ public class AbstractDelegatorTests extends TestCase { /** * Exception scenario - target method is called with invalid arguments. */ + @Test public void testInvalidArgumentsForExistingMethod() throws Exception { delegator.setTargetMethod("setName"); delegator.afterPropertiesSet(); try { delegator.invokeDelegateMethodWithArgument(new Object()); - fail(); + fail("Expected IllegalArgumentException"); } - catch (DynamicMethodInvocationException e) { + catch (IllegalArgumentException e) { // expected } } @@ -133,31 +143,32 @@ public class AbstractDelegatorTests extends TestCase { * Exception scenario - target method is called with incorrect number of * arguments. */ - public void testIncorrectArgumentCount() throws Exception { + @Test + public void testTooFewArguments() throws Exception { delegator.setTargetMethod("setName"); delegator.afterPropertiesSet(); try { // single argument expected but none provided delegator.invokeDelegateMethod(); - fail(); + fail("Expected IllegalArgumentException"); } - catch (DynamicMethodInvocationException e) { + catch (IllegalArgumentException e) { // expected } + } - try { - // single argument expected but two provided - delegator.invokeDelegateMethodWithArguments(new Object[] { "name", "anotherName" }); - fail(); - } - catch (DynamicMethodInvocationException e) { - // expected - } + @Test + public void testTooManyArguments() throws Exception { + delegator.setTargetMethod("setName"); + // single argument expected but two provided + delegator.invokeDelegateMethodWithArguments(new Object[] { "name", "anotherName" }); + assertEquals("name", foo.getName()); } /** * Exception scenario - incorrect static arguments set. */ + @Test public void testIncorrectNumberOfStaticArguments() throws Exception { delegator.setTargetMethod("setName"); @@ -187,6 +198,7 @@ public class AbstractDelegatorTests extends TestCase { * exception. Such 'business' exception should be re-thrown as is (without * wrapping). */ + @Test public void testDelegateException() throws Exception { delegator.setTargetMethod("fail"); delegator.afterPropertiesSet(); @@ -199,10 +211,12 @@ public class AbstractDelegatorTests extends TestCase { } } - + /** - * Exception scenario - target method is successfully invoked but throws a {@link Throwable} (not an {@link Exception}). + * Exception scenario - target method is successfully invoked but throws a + * {@link Throwable} (not an {@link Exception}). */ + @Test public void testDelegateThrowable() throws Exception { delegator.setTargetMethod("failUgly"); delegator.afterPropertiesSet(); @@ -215,4 +229,62 @@ public class AbstractDelegatorTests extends TestCase { } } + @SuppressWarnings("unused") + private static class Foo { + + public static final String FAILURE_MESSAGE = "Foo Failure!"; + + public static final String UGLY_FAILURE_MESSAGE = "Ugly Foo Failure!"; + + private String name; + + private int value; + + public Foo(String name, int value) { + this.name = name; + this.value = value; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public int getValue() { + return value; + } + + public void fail() throws Exception { + throw new Exception(FAILURE_MESSAGE); + } + + public void failUgly() throws Throwable { + throw new Throwable(UGLY_FAILURE_MESSAGE); + } + + } + + private static class FooService { + + private List processedFooNameValuePairs = new ArrayList(); + + @SuppressWarnings("unused") + public void processNameValuePair(String name, int value) { + processedFooNameValuePairs.add(new Foo(name, value)); + } + + @SuppressWarnings("unused") + public void processNameValuePair(String name, String value) { + processedFooNameValuePairs.add(new Foo(name, new Integer(value))); + } + + public List getProcessedFooNameValuePairs() { + return processedFooNameValuePairs; + } + + } + }