From 086798a0d41ad954b0540f9e2d114ce07b055db6 Mon Sep 17 00:00:00 2001 From: felmer <felmer> Date: Wed, 27 Aug 2008 06:19:55 +0000 Subject: [PATCH] bug found, test written and fixed: BeanUtil couldn't handle cyclic dependent object graphs SVN: 8083 --- .../cisd/common/utilities/BeanUtils.java | 67 ++++--- .../cisd/common/utilities/BeanUtilsTest.java | 186 +++++++++++++++++- 2 files changed, 227 insertions(+), 26 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 701e5ec153b..da70861a925 100644 --- a/common/source/java/ch/systemsx/cisd/common/utilities/BeanUtils.java +++ b/common/source/java/ch/systemsx/cisd/common/utilities/BeanUtils.java @@ -276,7 +276,8 @@ public final class BeanUtils public static <T> T fillBean(final Class<T> beanClass, final T beanInstance, final Object sourceBean) { - return fillBean(beanClass, beanInstance, sourceBean, EMPTY_ANNOTATION_MAP, NULL_CONVERTER); + return fillBean(beanClass, beanInstance, new HashMap<Object, Object>(), sourceBean, + EMPTY_ANNOTATION_MAP, NULL_CONVERTER); } /** @@ -286,7 +287,8 @@ public final class BeanUtils */ public static <T> T createBean(final Class<T> beanClass, final Object sourceBean) { - return fillBean(beanClass, null, sourceBean, EMPTY_ANNOTATION_MAP, NULL_CONVERTER); + return fillBean(beanClass, null, new HashMap<Object, Object>(), sourceBean, + EMPTY_ANNOTATION_MAP, NULL_CONVERTER); } /** @@ -311,7 +313,8 @@ public final class BeanUtils { c = NULL_CONVERTER; } - return fillBean(beanClass, beanInstance, sourceBean, EMPTY_ANNOTATION_MAP, c); + return fillBean(beanClass, beanInstance, new HashMap<Object, Object>(), sourceBean, + EMPTY_ANNOTATION_MAP, c); } /** @@ -328,13 +331,19 @@ public final class BeanUtils */ public static <T> T createBean(final Class<T> beanClass, final Object sourceBean, final Converter converter) + { + return createBean(beanClass, new HashMap<Object, Object>(), sourceBean, converter); + } + + private static <T> T createBean(final Class<T> beanClass, final Map<Object, Object> repository, + final Object sourceBean, final Converter converter) { Converter c = converter; if (c == null) { c = NULL_CONVERTER; } - return fillBean(beanClass, null, sourceBean, EMPTY_ANNOTATION_MAP, c); + return fillBean(beanClass, null, repository, sourceBean, EMPTY_ANNOTATION_MAP, c); } /** @@ -353,7 +362,7 @@ public final class BeanUtils * @return The new bean or <code>null</code> if <var>sourceBean</var> is <code>null</code>. */ @SuppressWarnings("unchecked") - private static <T> T fillBean(final Class<T> beanClass, final T beanInstance, + private static <T> T fillBean(final Class<T> beanClass, final T beanInstance, final Map<Object, Object> repository, final Object sourceBean, final AnnotationMap setterAnnotations, final Converter converter) { @@ -364,6 +373,11 @@ public final class BeanUtils if (sourceBean == null) { return null; + } + Object convertedBean = repository.get(sourceBean); + if (convertedBean != null) + { + return (T) convertedBean; } try @@ -371,31 +385,32 @@ public final class BeanUtils T destinationBean = beanInstance != null ? beanInstance : instantiateBean(beanClass, sourceBean, setterAnnotations); + repository.put(sourceBean, destinationBean); if (isArray(destinationBean)) { if (isArray(sourceBean)) { - destinationBean = copyArrayToArray(destinationBean, sourceBean, converter); + destinationBean = copyArrayToArray(destinationBean, repository, sourceBean, converter); } else if (isCollection(sourceBean)) { destinationBean = - (T) copyCollectionToArray(destinationBean, (Collection<?>) sourceBean, + (T) copyCollectionToArray(destinationBean, repository, (Collection<?>) sourceBean, converter); } } else if (isCollection(destinationBean)) { if (isArray(sourceBean)) { - copyArrayToCollection((Collection<?>) destinationBean, sourceBean, + copyArrayToCollection((Collection<?>) destinationBean, repository, sourceBean, setterAnnotations, converter); } else if (isCollection(sourceBean)) { - copyCollectionToCollection((Collection<?>) destinationBean, + copyCollectionToCollection((Collection<?>) destinationBean, repository, (Collection<?>) sourceBean, setterAnnotations, converter); } } else { - copyBean(destinationBean, sourceBean, converter); + copyBean(destinationBean, repository, sourceBean, converter); } return destinationBean; } catch (final InvocationTargetException ex) @@ -536,8 +551,8 @@ public final class BeanUtils } @SuppressWarnings("unchecked") - private final static <T> T copyArrayToArray(final T destination, final Object source, - final Converter converter) throws IllegalAccessException, InvocationTargetException + private final static <T> T copyArrayToArray(final T destination, final Map<Object, Object> repository, + final Object source, final Converter converter) throws IllegalAccessException, InvocationTargetException { if (destination == null) { @@ -572,7 +587,7 @@ public final class BeanUtils { final Object sourceElement = Array.get(source, index); final Object destinationElement = - createBean(componentType, sourceElement, converter); + createBean(componentType, repository, sourceElement, converter); Array.set(returned, index, destinationElement); } } @@ -580,7 +595,7 @@ public final class BeanUtils } @SuppressWarnings("unchecked") - private final static <T> T[] copyCollectionToArray(final Object destination, + private final static <T> T[] copyCollectionToArray(final Object destination, final Map<Object, Object> repository, final Collection<T> source, final Converter converter) throws IllegalAccessException, InvocationTargetException { @@ -611,14 +626,14 @@ public final class BeanUtils for (final Object sourceElement : source) { final Object destinationElement = - createBean(componentType, sourceElement, converter); + createBean(componentType, repository, sourceElement, converter); Array.set(returned, index++, destinationElement); } } return returned; } - private static void copyArrayToCollection(final Collection<?> destination, final Object source, + private static void copyArrayToCollection(final Collection<?> destination, final Map<Object, Object> repository, final Object source, final AnnotationMap setterAnnotations, final Converter converter) { if (destination == null) @@ -640,13 +655,13 @@ public final class BeanUtils { final Object sourceElement = Array.get(source, index); final Object destinationElement = - createBean(componentType, sourceElement, converter); + createBean(componentType, repository, sourceElement, converter); addToUntypedCollection(destination, destinationElement); } } } - private static void copyCollectionToCollection(final Collection<?> destination, + private static void copyCollectionToCollection(final Collection<?> destination, final Map<Object, Object> repository, final Collection<?> source, final AnnotationMap setterAnnotations, final Converter converter) { @@ -666,7 +681,7 @@ public final class BeanUtils for (final Object sourceElement : source) { final Object destinationElement = - createBean(componentType, sourceElement, converter); + createBean(componentType, repository, sourceElement, converter); addToUntypedCollection(destination, destinationElement); } } @@ -694,7 +709,7 @@ public final class BeanUtils return mapping; } - private static <T> void copyBean(final T destination, final Object source, + private static <T> void copyBean(final T destination, final Map<Object, Object> repository, final Object source, final Converter converter) throws IllegalAccessException, InvocationTargetException { if (destination == null) @@ -711,7 +726,7 @@ public final class BeanUtils for (final Method setter : destinationSetters) { final T newBean = - emergeNewBean(setter, source, destination, sourceGetters, destinationGetters, + emergeNewBean(setter, source, repository, destination, sourceGetters, destinationGetters, converter); if (newBean != null) { @@ -754,9 +769,10 @@ public final class BeanUtils */ @SuppressWarnings("unchecked") private static <T> T emergeNewBean(final Method setter, final Object source, - final T destination, final Map<String, Method> sourceGetters, - final Map<String, Method> destinationGetters, final Converter converter) - throws IllegalArgumentException, IllegalAccessException, InvocationTargetException + final Map<Object, Object> repository, final T destination, + final Map<String, Method> sourceGetters, final Map<String, Method> destinationGetters, + final Converter converter) throws IllegalArgumentException, IllegalAccessException, + InvocationTargetException { final AnnotationMap annotationMap = new SetterAnnotationMap(setter); final Method converterMethod = getConverterMethod(setter, source, converter); @@ -781,7 +797,8 @@ public final class BeanUtils // <code>destinationOldBean</code>, // then take it. final T destinationOldBean = (T) getOldBean(setter, destinationGetters, destination); - return fillBean(parameterType, destinationOldBean, oldBean, annotationMap, converter); + return fillBean(parameterType, destinationOldBean, repository, oldBean, annotationMap, + converter); } } 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 2728f7116a7..ae13b64e05f 100644 --- a/common/sourceTest/java/ch/systemsx/cisd/common/utilities/BeanUtilsTest.java +++ b/common/sourceTest/java/ch/systemsx/cisd/common/utilities/BeanUtilsTest.java @@ -647,7 +647,7 @@ public final class BeanUtilsTest this.bean = bean; } } - + @Test public void testFillComplexBeanWithNull() { @@ -1057,4 +1057,188 @@ public final class BeanUtilsTest final BeanWithEnum bean2 = BeanUtils.createBean(BeanWithEnum.class, bean1); assertEquals(State.BLOCKED, bean2.getState()); } + + public static class CyclicBeanA1 + { + private String name; + + private CyclicBeanB1 cyclicBeanB; + + private CyclicBeanA1[] cyclicBeans; + + public final String getName() + { + return name; + } + + public final void setName(String name) + { + this.name = name; + } + + public final CyclicBeanB1 getCyclicBeanB() + { + return cyclicBeanB; + } + + public final void setCyclicBeanB(CyclicBeanB1 cyclicBeanB) + { + this.cyclicBeanB = cyclicBeanB; + } + + public final CyclicBeanA1[] getCyclicBeans() + { + return cyclicBeans; + } + + public final void setCyclicBeans(CyclicBeanA1... cyclicBeans) + { + this.cyclicBeans = cyclicBeans; + } + } + + public static class CyclicBeanB1 + { + private String name; + + private CyclicBeanA1 cyclicBeanA; + + private List<CyclicBeanB1> cyclicBeans; + + public final String getName() + { + return name; + } + + public final void setName(String name) + { + this.name = name; + } + + public final CyclicBeanA1 getCyclicBeanA() + { + return cyclicBeanA; + } + + public final void setCyclicBeanA(CyclicBeanA1 cyclicBeanA) + { + this.cyclicBeanA = cyclicBeanA; + } + + public final List<CyclicBeanB1> getCyclicBeans() + { + return cyclicBeans; + } + + public final void setCyclicBeans(List<CyclicBeanB1> cyclicBeans) + { + this.cyclicBeans = cyclicBeans; + } + } + + public static class CyclicBeanA2 + { + private String name; + + private CyclicBeanB2 cyclicBeanB; + + private CyclicBeanA2[] cyclicBeans; + + public final String getName() + { + return name; + } + + public final void setName(String name) + { + this.name = name; + } + + public final CyclicBeanB2 getCyclicBeanB() + { + return cyclicBeanB; + } + + public final void setCyclicBeanB(CyclicBeanB2 cyclicBeanB) + { + this.cyclicBeanB = cyclicBeanB; + } + + public final CyclicBeanA2[] getCyclicBeans() + { + return cyclicBeans; + } + + public final void setCyclicBeans(CyclicBeanA2... cyclicBeans) + { + this.cyclicBeans = cyclicBeans; + } + } + + public static class CyclicBeanB2 + { + private String name; + + private CyclicBeanA2 cyclicBeanA; + + private List<CyclicBeanB2> cyclicBeans; + + public final String getName() + { + return name; + } + + public final void setName(String name) + { + this.name = name; + } + + public final CyclicBeanA2 getCyclicBeanA() + { + return cyclicBeanA; + } + + public final void setCyclicBeanA(CyclicBeanA2 cyclicBeanA) + { + this.cyclicBeanA = cyclicBeanA; + } + + public final List<CyclicBeanB2> getCyclicBeans() + { + return cyclicBeans; + } + + @CollectionMapping(collectionClass = ArrayList.class, elementClass = CyclicBeanB2.class) + public final void setCyclicBeans(List<CyclicBeanB2> cyclicBeans) + { + this.cyclicBeans = cyclicBeans; + } + } + + @Test + public void testConversionOfCyclicBeans() + { + CyclicBeanA1 cyclicBeanA1 = new CyclicBeanA1(); + CyclicBeanB1 cyclicBeanB1 = new CyclicBeanB1(); + cyclicBeanA1.setName("a"); + cyclicBeanA1.setCyclicBeanB(cyclicBeanB1); + cyclicBeanA1.setCyclicBeans(cyclicBeanA1); + cyclicBeanB1.setName("b"); + cyclicBeanB1.setCyclicBeanA(cyclicBeanA1); + List<CyclicBeanB1> beans = new ArrayList<CyclicBeanB1>(); + beans.add(cyclicBeanB1); + cyclicBeanB1.setCyclicBeans(beans); + + CyclicBeanA2 cyclicBeanA2 = BeanUtils.createBean(CyclicBeanA2.class, cyclicBeanA1); + assertEquals("a", cyclicBeanA2.getName()); + CyclicBeanB2 cyclicBeanB2 = cyclicBeanA2.getCyclicBeanB(); + assertEquals("b", cyclicBeanB2.getName()); + assertSame(cyclicBeanA2, cyclicBeanB2.getCyclicBeanA()); + assertEquals(1, cyclicBeanA2.getCyclicBeans().length); + assertEquals("a", cyclicBeanA2.getCyclicBeans()[0].getName()); + assertSame(cyclicBeanA2, cyclicBeanA2.getCyclicBeans()[0]); + assertEquals(1, cyclicBeanB2.getCyclicBeans().size()); + assertEquals("b", cyclicBeanB2.getCyclicBeans().get(0).getName()); + assertSame(cyclicBeanB2, cyclicBeanB2.getCyclicBeans().get(0)); + } } \ No newline at end of file -- GitLab