bug found, test written and fixed: BeanUtil couldn't handle cyclic dependent object graphs

SVN: 8083

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);
         }
     }
 

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