Fixes up some cloning with value types and collections

Shazwazza · Shazwazza · commit 7aa1f480c971 · 2014-04-16T15:33:21.000+10:00
diff --git a/src/Umbraco.Core/Models/DeepCloneHelper.cs b/src/Umbraco.Core/Models/DeepCloneHelper.cs
@@ -48,13 +48,13 @@ public static void DeepCloneRefProperties(IDeepCloneable input, IDeepCloneable o
                 {
                     IList newList;
                     if (propertyInfo.PropertyType.IsGenericType
-                        && (propertyInfo.PropertyType.GetGenericTypeDefinition() == typeof (IEnumerable<>)
-                            || propertyInfo.PropertyType.GetGenericTypeDefinition() == typeof (ICollection<>)
+                        && (propertyInfo.PropertyType.GetGenericTypeDefinition() == typeof(IEnumerable<>)
+                            || propertyInfo.PropertyType.GetGenericTypeDefinition() == typeof(ICollection<>)
                             || propertyInfo.PropertyType.GetGenericTypeDefinition() == typeof(IList<>)))
                     {
                         //if it is a IEnumerable<>, IList<T> or ICollection<> we'll use a List<>
-                        var genericType = typeof (List<>).MakeGenericType(propertyInfo.PropertyType.GetGenericArguments());
-                        newList = (IList) Activator.CreateInstance(genericType);
+                        var genericType = typeof(List<>).MakeGenericType(propertyInfo.PropertyType.GetGenericArguments());
+                        newList = (IList)Activator.CreateInstance(genericType);
                     }
                     else if (propertyInfo.PropertyType.IsArray
                              || (propertyInfo.PropertyType.IsInterface && propertyInfo.PropertyType.IsGenericType == false))
@@ -86,22 +86,37 @@ public static void DeepCloneRefProperties(IDeepCloneable input, IDeepCloneable o
                     var enumerable = (IEnumerable)propertyInfo.GetValue(input, null);
                     if (enumerable == null) continue;
 
-                    var isDeepClonableItems = false;
+                    var isUsableType = true;
+
+                    //now clone each item
                     foreach (var o in enumerable)
                     {
+                        //first check if the item is deep cloneable and copy that way
                         var dc = o as IDeepCloneable;
                         if (dc != null)
                         {
-                            isDeepClonableItems = true;
                             newList.Add(dc.DeepClone());
                         }
-                        else if (isDeepClonableItems)
+                        else if (o is string || o.GetType().IsValueType)
+                        {
+                            //check if the item is a value type or a string, then we can just use it                         
+                            newList.Add(o);
+                        }
+                        else
                         {
-                            //if not all items are deep cloneable throw an exception
-                            throw new InvalidOperationException("Cannot deep clone items in a collection that are not all " + typeof(IDeepCloneable));
+                            //this will occur if the item is not a string or value type or IDeepCloneable, in this case we cannot
+                            // clone each element, we'll need to skip this property, people will have to manually clone this list
+                            isUsableType = false;
+                            break;
                         }
                     }
 
+                    //if this was not usable, skip this property
+                    if (isUsableType == false)
+                    {
+                        continue;
+                    }
+
                     if (propertyInfo.PropertyType.IsArray)
                     {
                         //need to convert to array
diff --git a/src/Umbraco.Tests/Models/DeepCloneHelperTests.cs b/src/Umbraco.Tests/Models/DeepCloneHelperTests.cs
@@ -146,11 +146,19 @@ public void Cannot_Deep_Clone_Collection_Properties_That_Are_Not_Cloneable()
         {
             var test1 = new Test3()
             {
-                MyTest1 = new object[] { new Test1(), "hello" }
+                MyTest1 = new object[]
+                {
+                    new Test1(), "hello",
+                    //not cloneable so this property will get skipped
+                    new Test2()
+                }
             };
 
-            Assert.Throws<InvalidOperationException>(() => test1.DeepClone());
-            
+            var clone = (Test3)test1.DeepClone();
+
+            //it skipped this property so these will now be the same
+            Assert.AreSame(clone.MyTest1, test1.MyTest1);
+
         }
 
         public class Test1 : BaseCloneable
diff --git a/src/Umbraco.Tests/Models/UserTests.cs b/src/Umbraco.Tests/Models/UserTests.cs
@@ -28,8 +28,8 @@ public void Can_Deep_Clone()
                 LastLockoutDate = DateTime.Now,
                 LastLoginDate = DateTime.Now,
                 LastPasswordChangeDate = DateTime.Now,
-                Password = "test pass",
-                PasswordAnswer = "answer",
+                //Password = "test pass",
+                //PasswordAnswer = "answer",
                 PasswordQuestion = "question",
                 //ProviderUserKey = "user key",
                 SessionTimeout = 5,
diff --git a/src/Umbraco.Tests/Models/UserTypeTests.cs b/src/Umbraco.Tests/Models/UserTypeTests.cs
@@ -1,4 +1,5 @@
 using System;
+using System.Linq;
 using NUnit.Framework;
 using Umbraco.Core.Models.Membership;
 

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,5 @@`
`1`	`1`	`using System;`
	`2`	`+using System.Linq;`
`2`	`3`	`using NUnit.Framework;`
`3`	`4`	`using Umbraco.Core.Models.Membership;`
`4`	`5`