From 8e9a9dc996847fbae8bd4358a46cdd9423bb426f Mon Sep 17 00:00:00 2001
From: Shannon <sdeminick@gmail.com>
Date: Fri, 9 May 2014 13:25:18 +1000
Subject: [PATCH] Fixes deep clone of User object

---
 src/Umbraco.Core/Models/Membership/User.cs | 40 ++++++++++++++++------
 src/Umbraco.Tests/Models/UserTests.cs      | 12 +++++++
 2 files changed, 42 insertions(+), 10 deletions(-)

diff --git a/src/Umbraco.Core/Models/Membership/User.cs b/src/Umbraco.Core/Models/Membership/User.cs
index 9c710f4375..3e4daa4473 100644
--- a/src/Umbraco.Core/Models/Membership/User.cs
+++ b/src/Umbraco.Core/Models/Membership/User.cs
@@ -59,9 +59,9 @@ namespace Umbraco.Core.Models.Membership
         private IUserType _userType;
         private string _name;
         private Type _userTypeKey;
-        private readonly List<string> _addedSections;
-        private readonly List<string> _removedSections;
-        private readonly ObservableCollection<string> _sectionCollection;
+        private List<string> _addedSections;
+        private List<string> _removedSections;
+        private ObservableCollection<string> _sectionCollection;
         private int _sessionTimeout;
         private int _startContentId;
         private int _startMediaId;
@@ -423,24 +423,44 @@ namespace Umbraco.Core.Models.Membership
 
             if (e.Action == NotifyCollectionChangedAction.Add)
             {
+                var item = e.NewItems.Cast<string>().First();
+
                 //remove from the removed/added sections (since people could add/remove all they want in one request)
-                _removedSections.RemoveAll(s => s == e.NewItems.Cast<string>().First());
-                _addedSections.RemoveAll(s => s == e.NewItems.Cast<string>().First());
+                _removedSections.Remove(item);
+                _addedSections.Remove(item);
 
                 //add to the added sections
-                _addedSections.Add(e.NewItems.Cast<string>().First());
+                _addedSections.Add(item);
             }
             else if (e.Action == NotifyCollectionChangedAction.Remove)
             {
+                var item = e.OldItems.Cast<string>().First();
+
                 //remove from the removed/added sections (since people could add/remove all they want in one request)
-                _removedSections.RemoveAll(s => s == e.OldItems.Cast<string>().First());
-                _addedSections.RemoveAll(s => s == e.OldItems.Cast<string>().First());
+                _removedSections.Remove(item);
+                _addedSections.Remove(item);
 
                 //add to the added sections
-                _removedSections.Add(e.OldItems.Cast<string>().First());
+                _removedSections.Add(item);
             }
         }
-        
+
+        public override object DeepClone()
+        {
+            var clone = (User)base.DeepClone();
+
+            //need to create new collections otherwise they'll get copied by ref
+            clone._addedSections = new List<string>();
+            clone._removedSections = new List<string>();
+            clone._sectionCollection = new ObservableCollection<string>();
+            //re-create the event handler
+            clone._sectionCollection.CollectionChanged += clone.SectionCollectionChanged;
+
+            clone.ResetDirtyProperties(false);
+
+            return clone;
+        }
+
         /// <summary>
         /// Internal class used to wrap the user in a profile
         /// </summary>
diff --git a/src/Umbraco.Tests/Models/UserTests.cs b/src/Umbraco.Tests/Models/UserTests.cs
index 84b0fe317d..94eadf7996 100644
--- a/src/Umbraco.Tests/Models/UserTests.cs
+++ b/src/Umbraco.Tests/Models/UserTests.cs
@@ -1,4 +1,5 @@
 using System;
+using System.Linq;
 using NUnit.Framework;
 using Umbraco.Core.Models.Membership;
 using Umbraco.Core.Serialization;
@@ -53,6 +54,17 @@ namespace Umbraco.Tests.Models
             {
                 Assert.AreEqual(propertyInfo.GetValue(clone, null), propertyInfo.GetValue(item, null));
             }
+
+            //ensure internal collections are differet
+            Assert.AreNotSame(item.AddedSections, clone.AddedSections);
+            Assert.AreNotSame(item.RemovedSections, clone.RemovedSections);
+
+            //ensure event handlers are still wired on clone
+            clone.AddAllowedSection("blah");
+            Assert.AreEqual(1, clone.AddedSections.Count());
+            clone.RemoveAllowedSection("blah");
+            Assert.AreEqual(1, clone.RemovedSections.Count());
+
         }
 
         [Test]