From 1ff56a93cdda7571be030bd0f58ce701bffbc7dc Mon Sep 17 00:00:00 2001 From: ribeaudc <ribeaudc> Date: Fri, 7 Sep 2007 08:59:19 +0000 Subject: [PATCH] remove: - Unused methods (at the same time related TODOs) add: - more unit tests change: - fillBean did not work correctly with bean (collection of beans) inside bean SVN: 1633 --- .../cisd/common/utilities/BeanUtils.java | 162 +++++------ .../cisd/common/utilities/BeanUtilsTest.java | 259 ++++++++---------- 2 files changed, 177 insertions(+), 244 deletions(-) diff --git a/common/source/java/ch/systemsx/cisd/common/utilities/BeanUtils.java b/common/source/java/ch/systemsx/cisd/common/utilities/BeanUtils.java index 3b2c5b9e65c..0a3d18bcb67 100644 --- a/common/source/java/ch/systemsx/cisd/common/utilities/BeanUtils.java +++ b/common/source/java/ch/systemsx/cisd/common/utilities/BeanUtils.java @@ -64,85 +64,6 @@ public final class BeanUtils // Can not be instantiated. } - /** - * Checks the list of bean objects item by item for public getters which return <code>null</code> or 0. - * - * @param beanListToCheck The list of beans to check. Can be <code>null</code>. - * @return <var>beanListToCheck</var> (the parameter itself) - * @see #checkGettersNotNull(Object) - * @throws IllegalStateException If at least one of the public getters returns <code>null</code> or 0. - */ - public final static <T> List<T> checkGettersNotNull(List<T> beanListToCheck) - { - if (beanListToCheck == null) - { - return beanListToCheck; - } - for (Object bean : beanListToCheck) - { - checkGettersNotNull(bean); - } - return beanListToCheck; - } - - /** - * Checks bean object for public getters which return <code>null</code> or 0. - * - * @param beanToCheck The bean to check. Can be <code>null</code>. Must not be an array type. - * @return <var>beanToCheck</var> (the parameter itself) - * @throws IllegalArgumentException If the <var>beanToCheck</var> is an array type. - * @throws IllegalStateException If at least one of the public getters returns <code>null</code> or 0. - */ - public final static <T> T checkGettersNotNull(T beanToCheck) - { - if (beanToCheck == null) - { - return beanToCheck; - } - // TODO 2007-07-11, Franz-Josef Elmer: Why arrays are not checked? Why they are forbidden and not ignored? - if (beanToCheck.getClass().isArray()) - { - throw new IllegalArgumentException("Arrays are not supported."); - } - for (Method method : beanToCheck.getClass().getMethods()) - { - if (method.getName().startsWith(GETTER_PREFIX) && method.getParameterTypes().length == 0 - && Modifier.isPublic(method.getModifiers())) - { - try - { - final Object result = method.invoke(beanToCheck, ArrayUtils.EMPTY_OBJECT_ARRAY); - if (result == null) - { - throw new IllegalStateException("Method '" + method.getName() + "' returns null."); - } else if (isNull(result)) - { - throw new IllegalStateException("Method '" + method.getName() + "' returns 0."); - } - } catch (InvocationTargetException ex) - { - final Throwable cause = ex.getCause(); - if (cause instanceof Error) - { - throw (Error) cause; - } - throw CheckedExceptionTunnel.wrapIfNecessary((Exception) cause); - } catch (IllegalAccessException ex) - { - // Can't happen since we checked for isPublic() - throw new Error("Cannot call method '" + method.getName() + "'."); - } - } - } - return beanToCheck; - } - - // TODO 2007-07-11, Franz-Josef Elmer: Why numbers with a value rounded to 0 are forbidden? - private static boolean isNull(Object objectToCheck) - { - return (objectToCheck instanceof Number) && ((Number) objectToCheck).longValue() == 0; - } - /** * A map that provides annotations for given annotation classes. */ @@ -356,13 +277,13 @@ public final class BeanUtils try { - final T destinationBean = + T destinationBean = (beanInstance != null) ? beanInstance : instantiateBean(beanClass, sourceBean, setterAnnotations); if (isArray(destinationBean)) { if (isArray(sourceBean)) { - copyArrayToArray(destinationBean, sourceBean, converter); + destinationBean = copyArrayToArray(destinationBean, sourceBean, converter); } else if (isCollection(sourceBean)) { copyCollectionToArray(destinationBean, (Collection<?>) sourceBean, converter); @@ -502,26 +423,35 @@ public final class BeanUtils return (T) collectionClazz.newInstance(); } - private static void copyArrayToArray(Object destination, Object source, Converter converter) + @SuppressWarnings("unchecked") + private static <T> T copyArrayToArray(T destination, Object source, Converter converter) throws IllegalAccessException, InvocationTargetException { if (destination == null) { - return; + return null; } final Class<?> componentType = destination.getClass().getComponentType(); - final int length = Array.getLength(destination); + final int length = Array.getLength(source); + final T returned; + if (Array.getLength(destination) < length) + { + returned = (T) Array.newInstance(componentType, length); + } else + { + returned = destination; + } if (immutableTypes.contains(componentType)) { if (componentType == source.getClass().getComponentType()) { - System.arraycopy(source, 0, destination, 0, length); + System.arraycopy(source, 0, returned, 0, length); } else { for (int index = 0; index < length; ++index) { final Object sourceElement = Array.get(source, index); - Array.set(destination, index, sourceElement); + Array.set(returned, index, sourceElement); } } } else @@ -530,9 +460,10 @@ public final class BeanUtils { final Object sourceElement = Array.get(source, index); final Object destinationElement = createBean(componentType, sourceElement, converter); - Array.set(destination, index, destinationElement); + Array.set(returned, index, destinationElement); } } + return returned; } private static void copyCollectionToArray(Object destination, Collection<?> source, Converter converter) @@ -635,7 +566,7 @@ public final class BeanUtils return mapping; } - private static void copyBean(Object destination, Object source, Converter converter) throws IllegalAccessException, + private static <T> void copyBean(T destination, Object source, Converter converter) throws IllegalAccessException, InvocationTargetException { if (destination == null) @@ -643,11 +574,12 @@ public final class BeanUtils return; } final Collection<Method> destinationSetters = scanForPublicMethods(destination, SETTER_PREFIX, 1).values(); + final Map<String, Method> destinationGetters = scanForPublicMethods(destination, GETTER_PREFIX, 0); final Map<String, Method> sourceGetters = scanForPublicMethods(source, GETTER_PREFIX, 0); scanForPublicMethods(source, sourceGetters, BOOLEAN_GETTER_PREFIX, 0, boolean.class, Boolean.class); for (Method setter : destinationSetters) { - final Object newBean = emergeNewBean(setter, source, sourceGetters, converter); + final T newBean = emergeNewBean(setter, source, destination, sourceGetters, destinationGetters, converter); if (newBean != null) { try @@ -671,30 +603,62 @@ public final class BeanUtils } } - private static Object emergeNewBean(Method setter, Object source, Map<String, Method> sourceGetters, - Converter converter) throws IllegalArgumentException, IllegalAccessException, InvocationTargetException + /** + * Emerges the new bean. + * <p> + * The logic of this method is the following: + * <ol> + * <li>If a converter could be found for transferring the value from source bean to destination bean, then use it</li> + * <li>If the value is of primitive type or one of the immutable types specified, then use it tel quel</li> + * <li>If the value is a complexe type, then it should be filled using + * {@link #fillBean(Class, Object, Object, ch.systemsx.cisd.common.utilities.BeanUtils.AnnotationMap, ch.systemsx.cisd.common.utilities.BeanUtils.Converter)} + * before using it</li> + * </ol> + * </p> + */ + @SuppressWarnings("unchecked") + private static <T> T emergeNewBean(Method setter, Object source, T destination, Map<String, Method> sourceGetters, + Map<String, Method> destinationGetters, Converter converter) throws IllegalArgumentException, + IllegalAccessException, InvocationTargetException { final AnnotationMap annotationMap = new SetterAnnotationMap(setter); final Method converterMethod = getConverterMethod(setter, source, converter); if (converterMethod != null) { - return converterMethod.invoke(converter, new Object[] + return (T) converterMethod.invoke(converter, new Object[] { source }); } - final Method getter = getGetter(setter, sourceGetters, annotationMap); - if (getter == null) + final Object oldBean = getOldBean(setter, sourceGetters, source); + if (oldBean == null) { return null; } - final Object oldBean = getter.invoke(source, ArrayUtils.EMPTY_OBJECT_ARRAY); - final Class<?> parameterType = setter.getParameterTypes()[0]; + final Class<T> parameterType = (Class<T>) setter.getParameterTypes()[0]; if (parameterType.isPrimitive() || immutableTypes.contains(parameterType)) { - return oldBean; + return (T) oldBean; } else { - return fillBean(parameterType, null, oldBean, annotationMap, converter); + // If a non-null value could be found in the destination bean for <code>destinationOldBean</code>, + // then take it. + T destinationOldBean = (T) getOldBean(setter, destinationGetters, destination); + return fillBean(parameterType, destinationOldBean, oldBean, annotationMap, converter); + } + } + + /** + * For given <var>setter</var> method, we try to get the corresponding <i>getter</i>. If this step succeeds, we + * invoke the found <i>getter</i> on given <var>bean</var> and returns the value. + */ + private final static Object getOldBean(Method setter, Map<String, Method> getters, Object bean) + throws IllegalArgumentException, IllegalAccessException, InvocationTargetException + { + final Method getter = getGetter(setter, getters); + if (getter == null) + { + return null; } + return getter.invoke(bean, ArrayUtils.EMPTY_OBJECT_ARRAY); } private static Method getConverterMethod(Method setter, Object sourceBean, Converter converter) @@ -720,7 +684,7 @@ public final class BeanUtils return null; } - private static Method getGetter(Method setter, Map<String, Method> sourceGetters, AnnotationMap annotationMap) + private static Method getGetter(Method setter, Map<String, Method> sourceGetters) { String propertyName = setter.getName().substring(SETTER_PREFIX.length()); final Method getter = sourceGetters.get(GETTER_PREFIX + propertyName); diff --git a/common/sourceTest/java/ch/systemsx/cisd/common/utilities/BeanUtilsTest.java b/common/sourceTest/java/ch/systemsx/cisd/common/utilities/BeanUtilsTest.java index 453b322be08..13f7ba59e18 100644 --- a/common/sourceTest/java/ch/systemsx/cisd/common/utilities/BeanUtilsTest.java +++ b/common/sourceTest/java/ch/systemsx/cisd/common/utilities/BeanUtilsTest.java @@ -18,8 +18,8 @@ package ch.systemsx.cisd.common.utilities; import static org.testng.AssertJUnit.assertEquals; import static org.testng.AssertJUnit.assertNotNull; -import static org.testng.AssertJUnit.assertNull; import static org.testng.AssertJUnit.assertSame; +import static org.testng.AssertJUnit.assertTrue; import java.beans.PropertyDescriptor; import java.lang.reflect.InvocationTargetException; @@ -33,7 +33,6 @@ import java.util.LinkedList; import java.util.List; import org.apache.commons.lang.StringUtils; -import org.testng.AssertJUnit; import org.testng.annotations.Test; import ch.systemsx.cisd.common.annotation.CollectionMapping; @@ -79,85 +78,6 @@ public final class BeanUtilsTest assertEquals(fooBean.getFoo(), description); } - private static class SimpleBean - { - private final int number; - - private final String string; - - SimpleBean(int number, String string) - { - this.number = number; - this.string = string; - } - - public int getNumber() - { - return number; - } - - public String getString() - { - return string; - } - - String getIgnoreThisBecauseItIsNotPublic() - { - AssertJUnit.fail("Should be ignore because not public"); - return null; - } - } - - @Test - public void testCheckGettersForNullOK() - { - final SimpleBean bean = new SimpleBean(1, ""); - assert BeanUtils.checkGettersNotNull(bean) == bean; - } - - @Test - public void testCheckGettersForNullOKNullBean() - { - assertNull(BeanUtils.checkGettersNotNull(null)); - } - - @Test(expectedExceptions = IllegalStateException.class) - public void testCheckGettersForNullStringNull() - { - final SimpleBean bean = new SimpleBean(1, null); - BeanUtils.checkGettersNotNull(bean); - } - - @Test(expectedExceptions = IllegalStateException.class) - public void testCheckGettersForNullInt0() - { - final SimpleBean bean = new SimpleBean(0, "test"); - BeanUtils.checkGettersNotNull(bean); - } - - @Test(expectedExceptions = IllegalArgumentException.class) - public void testCheckGettersForNullForbiddenArray() - { - BeanUtils.checkGettersNotNull(new Object[0]); - } - - @Test - public void testCheckGettersForNullListOK() - { - final SimpleBean bean1 = new SimpleBean(1, "test"); - final SimpleBean bean2 = new SimpleBean(5, "test2"); - final List<SimpleBean> beanList = Arrays.asList(bean1, bean2); - assert BeanUtils.checkGettersNotNull(beanList) == beanList; - } - - @Test(expectedExceptions = IllegalStateException.class) - public void testCheckGettersForNullListInt0() - { - final SimpleBean bean1 = new SimpleBean(1, "test"); - final SimpleBean bean2 = new SimpleBean(0, "test2"); - BeanUtils.checkGettersNotNull(Arrays.asList(bean1, bean2)); - } - public static class Bean1a { private int i; @@ -415,11 +335,7 @@ public final class BeanUtilsTest @Test public void testFillSimpleBean() { - final Bean1a b1 = new Bean1a(); - b1.setB(true); - b1.setF(0.2f); - b1.setI(17); - b1.setS("test"); + final Bean1a b1 = createBean1a(); b1.setBb(true); final Bean2a b2 = BeanUtils.createBean(Bean2a.class, b1); assertBeansAreEqual("Beans are not equal", b1, b2); @@ -428,11 +344,7 @@ public final class BeanUtilsTest @Test public void testFillPreinstantiatedBean() { - final Bean1a b1 = new Bean1a(); - b1.setB(true); - b1.setF(0.2f); - b1.setI(17); - b1.setS("test"); + final Bean1a b1 = createBean1a(); b1.setBb(true); final Bean2a b2 = new Bean2a(); assertSame(b2, BeanUtils.fillBean(Bean2a.class, b2, b1)); @@ -460,11 +372,7 @@ public final class BeanUtilsTest @Test public void testFillSimpleBeanWithNativeWrapper2() { - final Bean1a b1 = new Bean1a(); - b1.setB(true); - b1.setF(0.2f); - b1.setI(17); - b1.setS("test"); + final Bean1a b1 = createBean1a(); b1.setBb(true); final Bean2b b2 = BeanUtils.createBean(Bean2b.class, b1); assertEquals(b1.isB(), b2.isB().booleanValue()); @@ -494,11 +402,7 @@ public final class BeanUtilsTest @Test public void testFillSimpleBeanArray() { - final Bean1a b1a = new Bean1a(); - b1a.setB(true); - b1a.setF(0.2f); - b1a.setI(17); - b1a.setS("test"); + final Bean1a b1a = createBean1a(); final Bean1a b1b = new Bean1a(); b1b.setB(false); b1b.setF(0.3f); @@ -751,11 +655,7 @@ public final class BeanUtilsTest @Test public void testFillComplexBean() { - final Bean1a b1 = new Bean1a(); - b1.setB(true); - b1.setF(0.2f); - b1.setI(17); - b1.setS("test"); + final Bean1a b1 = createBean1a(); final BeanWithBean1 b3 = new BeanWithBean1(); b3.setBean(b1); final BeanWithBean2 b4 = BeanUtils.createBean(BeanWithBean2.class, b3); @@ -763,6 +663,32 @@ public final class BeanUtilsTest assertBeansAreEqual("Bean comparison", b1, b2); } + private final static Bean1a createBean1a() + { + final Bean1a b1 = new Bean1a(); + b1.setB(true); + b1.setF(0.2f); + b1.setI(17); + b1.setS("test"); + return b1; + } + + @Test(dependsOnMethods = "testFillComplexBean") + public final void testFillComplexBeanWithNonNullInnerBean() + { + final BeanWithBean1 b1 = new BeanWithBean1(); + Bean1a bean1a = createBean1a(); + b1.setBean(bean1a); + final BeanWithBean2 b2 = new BeanWithBean2(); + Bean2a bean2a = new Bean2a(); + b2.setBean(bean2a); + BeanUtils.fillBean(BeanWithBean2.class, b2, b1); + assertBeansAreEqual("Bean comparison", bean1a, b2.getBean()); + // Here is the main difference to 'testFillComplexBean': no new bean has been created but + // we 'recycled' the already present one. + assertTrue(bean2a == b2.getBean()); + } + public static class BeanWithBeanArray1 { private Bean1a[] bean; @@ -826,7 +752,7 @@ public final class BeanUtilsTest } @Test - public void testFillBeanWithBeanCollectionFromBeanWithBeanArray1() + public void testCreateBeanWithBeanCollectionFromBeanWithBeanArray1() { final Bean1a b1a = new Bean1a(); b1a.setB(true); @@ -854,13 +780,9 @@ public final class BeanUtilsTest } @Test - public void testFillBeanWithBeanCollectionFromBeanWithBeanArray2() + public void testCreateBeanWithBeanCollectionFromBeanWithBeanArray2() { - final Bean2a b2a = new Bean2a(); - b2a.setB(true); - b2a.setF(0.2f); - b2a.setI(17); - b2a.setS("test"); + final Bean2a b2a = createBean2a(); final Bean2a b2b = new Bean2a(); b2b.setB(false); b2b.setF(1.1f); @@ -881,19 +803,21 @@ public final class BeanUtilsTest } } + private final static Bean2a createBean2a() + { + final Bean2a b2a = new Bean2a(); + b2a.setB(true); + b2a.setF(0.2f); + b2a.setI(17); + b2a.setS("test"); + return b2a; + } + @Test - public void testFillBeanWithBeanArrayFromBeanWithBeanCollection() + public void testCreateBeanWithBeanArrayFromBeanWithBeanCollection() { - final Bean1a b1a = new Bean1a(); - b1a.setB(true); - b1a.setF(0.2f); - b1a.setI(17); - b1a.setS("test"); - final Bean1a b1b = new Bean1a(); - b1b.setB(false); - b1b.setF(1.1f); - b1b.setI(31); - b1b.setS("test2"); + final Bean1a b1a = createBean1a(); + final Bean1a b1b = createOtherBean1a(); final BeanWithBeanCollection1 b1Collection = new BeanWithBeanCollection1(); final Collection<Bean1a> colb1 = new LinkedHashSet<Bean1a>(Arrays.asList(new Bean1a[] { b1a, b1b })); @@ -910,19 +834,21 @@ public final class BeanUtilsTest } } - @Test - public void testFillBeanWithBeanArrayFromBeanWithBeanArray() + private final static Bean1a createOtherBean1a() { - final Bean1a b1a = new Bean1a(); - b1a.setB(true); - b1a.setF(0.2f); - b1a.setI(17); - b1a.setS("test"); final Bean1a b1b = new Bean1a(); b1b.setB(false); b1b.setF(1.1f); b1b.setI(31); b1b.setS("test2"); + return b1b; + } + + @Test + public void testCreateBeanWithBeanArrayFromBeanWithBeanArray() + { + final Bean1a b1a = createBean1a(); + final Bean1a b1b = createOtherBean1a(); final BeanWithBeanArray1 b1Array = new BeanWithBeanArray1(); final Bean1a[] arrayb1 = new Bean1a[] { b1a, b1b }; @@ -930,6 +856,11 @@ public final class BeanUtilsTest final BeanWithBeanArray2 b2Array = BeanUtils.createBean(BeanWithBeanArray2.class, b1Array); final Bean2a[] arrayb2 = b2Array.getBeanArray(); assertNotNull(arrayb2); + assertBeanArraysAreEqual(arrayb1, arrayb2); + } + + private final static void assertBeanArraysAreEqual(final Bean1a[] arrayb1, final Bean2a[] arrayb2) + { assertEquals(arrayb1.length, arrayb2.length); for (int i = 0; i < arrayb1.length; ++i) { @@ -938,18 +869,10 @@ public final class BeanUtilsTest } @Test - public void testFillBeanWithBeanCollectionFromBeanWithBeanCollection() + public void testCreateBeanWithBeanCollectionFromBeanWithBeanCollection() { - final Bean1a b1a = new Bean1a(); - b1a.setB(true); - b1a.setF(0.2f); - b1a.setI(17); - b1a.setS("test"); - final Bean1a b1b = new Bean1a(); - b1b.setB(false); - b1b.setF(1.1f); - b1b.setI(31); - b1b.setS("test2"); + final Bean1a b1a = createBean1a(); + final Bean1a b1b = createOtherBean1a(); final BeanWithBeanCollection1 b1Collection = new BeanWithBeanCollection1(); final Collection<Bean1a> colb1 = new LinkedHashSet<Bean1a>(Arrays.asList(new Bean1a[] { b1a, b1b })); @@ -957,6 +880,12 @@ public final class BeanUtilsTest final BeanWithBeanCollection2 b2Collection = BeanUtils.createBean(BeanWithBeanCollection2.class, b1Collection); final Collection<Bean2a> colb2 = b2Collection.getBeanArray(); assertNotNull(colb2); + assertBeanCollectionsAreEqual(colb1, colb2); + } + + private final static void assertBeanCollectionsAreEqual(final Collection<Bean1a> colb1, + final Collection<Bean2a> colb2) + { assertEquals(colb1.size(), colb2.size()); final Iterator<Bean2a> itb2 = colb2.iterator(); int i = 0; @@ -966,7 +895,47 @@ public final class BeanUtilsTest } } - private void assertBeansAreEqual(String msg, Bean1a b1, Bean2a b2) + @Test + public final void testFillBeanWithComplexBeanCollection() + { + final Bean1a b1a = createBean1a(); + final Bean1a b1b = createOtherBean1a(); + final BeanWithBeanCollection1 b1 = new BeanWithBeanCollection1(); + final Collection<Bean1a> colb1 = new LinkedHashSet<Bean1a>(Arrays.asList(new Bean1a[] + { b1a, b1b })); + b1.setBeanArray(colb1); + final BeanWithBeanCollection2 b2 = new BeanWithBeanCollection2(); + // With empty collection + b2.setBeanArray(new LinkedHashSet<Bean2a>(Arrays.asList(new Bean2a[0]))); + BeanUtils.fillBean(BeanWithBeanCollection2.class, b2, b1); + assertBeanCollectionsAreEqual(b1.getBeanArray(), b2.getBeanArray()); + // With null + b2.setBeanArray(null); + BeanUtils.fillBean(BeanWithBeanCollection2.class, b2, b1); + assertBeanCollectionsAreEqual(b1.getBeanArray(), b2.getBeanArray()); + } + + @Test + public final void testFillBeanWithComplexBeanArray() + { + final Bean1a b1a = createBean1a(); + final Bean1a b1b = createOtherBean1a(); + final BeanWithBeanArray1 b1 = new BeanWithBeanArray1(); + final Bean1a[] colb1 = new Bean1a[] + { b1a, b1b }; + b1.setBeanArray(colb1); + final BeanWithBeanArray2 b2 = new BeanWithBeanArray2(); + // With empty collection + b2.setBeanArray(new Bean2a[0]); + BeanUtils.fillBean(BeanWithBeanArray2.class, b2, b1); + assertBeanArraysAreEqual(b1.getBeanArray(), b2.getBeanArray()); + // With null + b2.setBeanArray(null); + BeanUtils.fillBean(BeanWithBeanArray2.class, b2, b1); + assertBeanArraysAreEqual(b1.getBeanArray(), b2.getBeanArray()); + } + + private final static void assertBeansAreEqual(String msg, Bean1a b1, Bean2a b2) { assertNotNull(msg, b1); assertNotNull(msg, b2); @@ -977,7 +946,7 @@ public final class BeanUtilsTest assertEquals(msg, b1.getBb(), b2.getBb()); } - private void assertBeansAreEqual(String msg, Bean2a b2, Bean1a b1) + private final static void assertBeansAreEqual(String msg, Bean2a b2, Bean1a b1) { assertNotNull(msg, b1); assertNotNull(msg, b2); @@ -1032,4 +1001,4 @@ public final class BeanUtilsTest }); assertEquals("some to Foo", toFooBean.getBar()); } -} +} \ No newline at end of file -- GitLab