Fixes issue with deep cloning property values (#7795)

src/Umbraco.Core/Models/Property.cs

@@ -50,7 +50,7 @@ namespace Umbraco.Core.Models
         /// <summary>
         /// Represents a property value.
         /// </summary>
-        public class PropertyValue
+        public class PropertyValue : IEquatable<PropertyValue>
         {
             // TODO: Either we allow change tracking at this class level, or we add some special change tracking collections to the Property
             // class to deal with change tracking which variants have changed
@@ -95,6 +95,26 @@ namespace Umbraco.Core.Models
             /// </summary>
             public PropertyValue Clone()
                 => new PropertyValue { _culture = _culture, _segment = _segment, PublishedValue = PublishedValue, EditedValue = EditedValue };
+
+            public override bool Equals(object obj)
+            {
+                return Equals(obj as PropertyValue);
+            }
+
+            public bool Equals(PropertyValue other)
+            {
+                return other != null &&
+                       _culture == other._culture &&
+                       _segment == other._segment;
+            }
+
+            public override int GetHashCode()
+            {
+                var hashCode = -1254204277;
+                hashCode = hashCode * -1521134295 + EqualityComparer<string>.Default.GetHashCode(_culture);
+                hashCode = hashCode * -1521134295 + EqualityComparer<string>.Default.GetHashCode(_segment);
+                return hashCode;
+            }
         }
 
         private static readonly DelegateEqualityComparer<object> PropertyValueComparer = new DelegateEqualityComparer<object>(
@@ -338,6 +358,20 @@ namespace Umbraco.Core.Models
 
             var clonedEntity = (Property)clone;
 
+            //manually clone _values, _pvalue, _vvalues
+            clonedEntity._values = _values?.Select(x => x.Clone()).ToList(); // all values get copied
+            clonedEntity._pvalue = _pvalue?.Clone();
+            // the tricky part here is that _values contains ALL values including the values in the _vvalues
+            // dictionary and they are by reference which is why we have equality overloads on PropertyValue
+            if (clonedEntity._vvalues != null)
+            {
+                clonedEntity._vvalues = new Dictionary<CompositeNStringNStringKey, PropertyValue>();
+                foreach (var item in _vvalues)
+                {
+                    clonedEntity._vvalues[item.Key] = clonedEntity._values.First(x => x.Equals(item.Value));
+                }
+            }
+
             //need to manually assign since this is a readonly property
             clonedEntity.PropertyType = (PropertyType) PropertyType.DeepClone();
         }

src/Umbraco.Tests/Services/ContentServiceTests.cs

@@ -1890,6 +1890,49 @@ namespace Umbraco.Tests.Services
             //Assert.AreNotEqual(content.Name, copy.Name);
         }
 
+        [Test]
+        public void Can_Copy_And_Modify_Content_With_Events()
+        {
+            // see https://github.com/umbraco/Umbraco-CMS/issues/5513
+
+            TypedEventHandler<IContentService, CopyEventArgs<IContent>> copying = (sender, args) =>
+            {
+                args.Copy.SetValue("title", "1");
+                args.Original.SetValue("title", "2");                
+            };
+
+            TypedEventHandler<IContentService, CopyEventArgs<IContent>> copied = (sender, args) =>
+            {
+                var copyVal = args.Copy.GetValue<string>("title");
+                var origVal = args.Original.GetValue<string>("title");
+
+                Assert.AreEqual("1", copyVal);
+                Assert.AreEqual("2", origVal);
+            };
+
+            try
+            {                
+                var contentService = ServiceContext.ContentService;
+                
+                ContentService.Copying += copying;
+                ContentService.Copied += copied;
+
+                var contentType = MockedContentTypes.CreateSimpleContentType();
+                ServiceContext.ContentTypeService.Save(contentType);
+                var content = MockedContent.CreateSimpleContent(contentType);
+                content.SetValue("title", "New Value");
+                contentService.Save(content);
+
+                var copy = contentService.Copy(content, content.ParentId, false, Constants.Security.SuperUserId);
+                Assert.AreEqual("1", copy.GetValue("title"));
+            }
+            finally
+            {
+                ContentService.Copying -= copying;
+                ContentService.Copied -= copied;
+            }
+        }
+
         [Test]
         public void Can_Copy_Recursive()
         {