diff --git a/src/Umbraco.Core/Models/DeepCloneHelper.cs b/src/Umbraco.Core/Models/DeepCloneHelper.cs index 6470de912b..453b455d4b 100644 --- a/src/Umbraco.Core/Models/DeepCloneHelper.cs +++ b/src/Umbraco.Core/Models/DeepCloneHelper.cs @@ -81,9 +81,10 @@ namespace Umbraco.Core.Models if (propertyInfo.PropertyType.IsGenericType && (propertyInfo.PropertyType.GetGenericTypeDefinition() == typeof(IEnumerable<>) || propertyInfo.PropertyType.GetGenericTypeDefinition() == typeof(ICollection<>) - || propertyInfo.PropertyType.GetGenericTypeDefinition() == typeof(IList<>))) + || propertyInfo.PropertyType.GetGenericTypeDefinition() == typeof(IList<>) + || propertyInfo.PropertyType.GetGenericTypeDefinition() == typeof(IReadOnlyCollection<>))) { - //if it is a IEnumerable<>, IList or ICollection<> we'll use a List<> + //if it is a IEnumerable<>, IReadOnlyCollection, IList or ICollection<> we'll use a List<> since it implements them all var genericType = typeof(List<>).MakeGenericType(propertyInfo.PropertyType.GetGenericArguments()); return new ClonePropertyInfo(propertyInfo) { GenericListType = genericType }; } diff --git a/src/Umbraco.Core/Models/Property.cs b/src/Umbraco.Core/Models/Property.cs index f51f70faf6..1d044aa20d 100644 --- a/src/Umbraco.Core/Models/Property.cs +++ b/src/Umbraco.Core/Models/Property.cs @@ -50,7 +50,7 @@ namespace Umbraco.Core.Models /// /// Represents a property value. /// - public class PropertyValue : IEquatable + public class PropertyValue : IDeepCloneable, IEquatable { // 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 @@ -96,6 +96,8 @@ namespace Umbraco.Core.Models public PropertyValue Clone() => new PropertyValue { _culture = _culture, _segment = _segment, PublishedValue = PublishedValue, EditedValue = EditedValue }; + public object DeepClone() => Clone(); + public override bool Equals(object obj) { return Equals(obj as PropertyValue); @@ -105,14 +107,18 @@ namespace Umbraco.Core.Models { return other != null && _culture == other._culture && - _segment == other._segment; + _segment == other._segment && + EqualityComparer.Default.Equals(EditedValue, other.EditedValue) && + EqualityComparer.Default.Equals(PublishedValue, other.PublishedValue); } public override int GetHashCode() { - var hashCode = -1254204277; + var hashCode = 1885328050; hashCode = hashCode * -1521134295 + EqualityComparer.Default.GetHashCode(_culture); hashCode = hashCode * -1521134295 + EqualityComparer.Default.GetHashCode(_segment); + hashCode = hashCode * -1521134295 + EqualityComparer.Default.GetHashCode(EditedValue); + hashCode = hashCode * -1521134295 + EqualityComparer.Default.GetHashCode(PublishedValue); return hashCode; } } @@ -358,20 +364,6 @@ 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(); - 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(); } diff --git a/src/Umbraco.Tests/Models/ContentTests.cs b/src/Umbraco.Tests/Models/ContentTests.cs index 3116087669..5bb30e1606 100644 --- a/src/Umbraco.Tests/Models/ContentTests.cs +++ b/src/Umbraco.Tests/Models/ContentTests.cs @@ -24,7 +24,6 @@ using Umbraco.Web.PropertyEditors; namespace Umbraco.Tests.Models { - [TestFixture] public class ContentTests : UmbracoTestBase { @@ -42,7 +41,7 @@ namespace Umbraco.Tests.Models // all this is required so we can validate properties... var editor = new TextboxPropertyEditor(Mock.Of()) { Alias = "test" }; - Composition.Register(_ => new DataEditorCollection(new [] { editor })); + Composition.Register(_ => new DataEditorCollection(new[] { editor })); Composition.Register(); var dataType = Mock.Of(); Mock.Get(dataType).Setup(x => x.Configuration).Returns(() => new object()); @@ -55,7 +54,7 @@ namespace Umbraco.Tests.Models var mediaTypeService = Mock.Of(); var memberTypeService = Mock.Of(); Composition.Register(_ => ServiceContext.CreatePartial(dataTypeService: dataTypeService, contentTypeBaseServiceProvider: new ContentTypeBaseServiceProvider(_contentTypeService, mediaTypeService, memberTypeService))); - + } [Test] @@ -458,7 +457,7 @@ namespace Umbraco.Tests.Models Assert.IsTrue(prop.WasPropertyDirty("Id")); } Assert.IsTrue(content.WasPropertyDirty("CultureInfos")); - foreach(var culture in content.CultureInfos) + foreach (var culture in content.CultureInfos) { Assert.IsTrue(culture.WasDirty()); Assert.IsTrue(culture.WasPropertyDirty("Name")); @@ -539,7 +538,7 @@ namespace Umbraco.Tests.Models var content = MockedContent.CreateTextpageContent(contentType, "Textpage", -1); // Act - content.PropertyValues(new { title = "This is the new title"}); + content.PropertyValues(new { title = "This is the new title" }); // Assert Assert.That(content.Properties.Any(), Is.True); @@ -603,13 +602,13 @@ namespace Umbraco.Tests.Models // Act contentType.PropertyGroups["Content"].PropertyTypes.Add(new PropertyType("test", ValueStorageType.Ntext, "subtitle") - { - Name = "Subtitle", - Description = "Optional subtitle", - Mandatory = false, - SortOrder = 3, - DataTypeId = -88 - }); + { + Name = "Subtitle", + Description = "Optional subtitle", + Mandatory = false, + SortOrder = 3, + DataTypeId = -88 + }); // Assert Assert.That(contentType.PropertyGroups["Content"].PropertyTypes.Count, Is.EqualTo(3)); @@ -626,9 +625,13 @@ namespace Umbraco.Tests.Models // Act var propertyType = new PropertyType("test", ValueStorageType.Ntext, "subtitle") - { - Name = "Subtitle", Description = "Optional subtitle", Mandatory = false, SortOrder = 3, DataTypeId = -88 - }; + { + Name = "Subtitle", + Description = "Optional subtitle", + Mandatory = false, + SortOrder = 3, + DataTypeId = -88 + }; contentType.PropertyGroups["Content"].PropertyTypes.Add(propertyType); var newProperty = new Property(propertyType); newProperty.SetValue("This is a subtitle Test"); @@ -650,14 +653,14 @@ namespace Umbraco.Tests.Models // Act var propertyType = new PropertyType("test", ValueStorageType.Ntext, "subtitle") - { - Name = "Subtitle", - Description = "Optional subtitle", - Mandatory = false, - SortOrder = 3, - DataTypeId = -88 - }; - var propertyGroup = new PropertyGroup(true) { Name = "Test Group", SortOrder = 3}; + { + Name = "Subtitle", + Description = "Optional subtitle", + Mandatory = false, + SortOrder = 3, + DataTypeId = -88 + }; + var propertyGroup = new PropertyGroup(true) { Name = "Test Group", SortOrder = 3 }; propertyGroup.PropertyTypes.Add(propertyType); contentType.PropertyGroups.Add(propertyGroup); var newProperty = new Property(propertyType); @@ -681,9 +684,13 @@ namespace Umbraco.Tests.Models // Act - note that the PropertyType's properties like SortOrder is not updated through the Content object var propertyType = new PropertyType("test", ValueStorageType.Ntext, "title") - { - Name = "Title", Description = "Title description added", Mandatory = false, SortOrder = 10, DataTypeId = -88 - }; + { + Name = "Title", + Description = "Title description added", + Mandatory = false, + SortOrder = 10, + DataTypeId = -88 + }; content.Properties.Add(new Property(propertyType)); // Assert @@ -911,13 +918,13 @@ namespace Umbraco.Tests.Models // Act var propertyType = new PropertyType("test", ValueStorageType.Ntext, "subtitle") - { - Name = "Subtitle", - Description = "Optional subtitle", - Mandatory = false, - SortOrder = 3, - DataTypeId = -88 - }; + { + Name = "Subtitle", + Description = "Optional subtitle", + Mandatory = false, + SortOrder = 3, + DataTypeId = -88 + }; contentType.PropertyGroups["Content"].PropertyTypes.Add(propertyType); // Assert diff --git a/src/Umbraco.Tests/Models/PropertyTests.cs b/src/Umbraco.Tests/Models/PropertyTests.cs new file mode 100644 index 0000000000..a95afcdd5c --- /dev/null +++ b/src/Umbraco.Tests/Models/PropertyTests.cs @@ -0,0 +1,63 @@ +using System; +using System.Linq; +using NUnit.Framework; +using Umbraco.Core.Models; +using Umbraco.Tests.Testing; + +namespace Umbraco.Tests.Models +{ + [TestFixture] + public class PropertyTests : UmbracoTestBase + { + [Test] + public void Can_Deep_Clone() + { + // needs to be within collection to support publishing + var ptCollection = new PropertyTypeCollection(true, new[] {new PropertyType("TestPropertyEditor", ValueStorageType.Nvarchar, "test") + { + Id = 3, + CreateDate = DateTime.Now, + DataTypeId = 5, + PropertyEditorAlias = "propTest", + Description = "testing", + Key = Guid.NewGuid(), + Mandatory = true, + Name = "Test", + PropertyGroupId = new Lazy(() => 11), + SortOrder = 9, + UpdateDate = DateTime.Now, + ValidationRegExp = "xxxx", + ValueStorageType = ValueStorageType.Nvarchar + }}); + + var property = new Property(123, ptCollection[0]) + { + CreateDate = DateTime.Now, + Id = 4, + Key = Guid.NewGuid(), + UpdateDate = DateTime.Now + }; + + property.SetValue("hello"); + property.PublishValues(); + + var clone = (Property)property.DeepClone(); + + Assert.AreNotSame(clone, property); + Assert.AreNotSame(clone.Values, property.Values); + Assert.AreNotSame(property.PropertyType, clone.PropertyType); + for (int i = 0; i < property.Values.Count; i++) + { + Assert.AreNotSame(property.Values.ElementAt(i), clone.Values.ElementAt(i)); + } + + + //This double verifies by reflection + var allProps = clone.GetType().GetProperties(); + foreach (var propertyInfo in allProps) + { + Assert.AreEqual(propertyInfo.GetValue(clone, null), propertyInfo.GetValue(property, null)); + } + } + } +} diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index 90f9303423..3c359cdde8 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -142,6 +142,7 @@ +