swcs/Umbraco-CMS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fixes issue with deep cloning Property

Browse Source 
main problem was that it was duplicating the _pvalue value when it shouldn't have. The real fix is that the Values property solves all these problems with it's setter but that wasn't being used by the DeepClone engine because that explicitly looks for certain collection types and IReadOnlyCollection wasn't one of them. With that included and PropertyValue marked as IDeepCloneable, it all just works. I've added tests.
This commit is contained in:
Shannon
2020-03-26 19:56:21 +11:00
parent df04ffa68d
commit d40b7087f4
5 changed files with 116 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Umbraco.Core/Models/DeepCloneHelper.cs
+3 -2
View File
@@ -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<T> or ICollection<> we'll use a List<>
//if it is a IEnumerable<>, IReadOnlyCollection<T>, IList<T> 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 };
}
src/Umbraco.Core/Models/Property.cs
+9 -17
View File
@@ -50,7 +50,7 @@ namespace Umbraco.Core.Models
/// <summary>
/// Represents a property value.
/// </summary>
public class PropertyValue : IEquatable<PropertyValue>
public class PropertyValue : IDeepCloneable, 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
@@ -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<object>.Default.Equals(EditedValue, other.EditedValue) &&
EqualityComparer<object>.Default.Equals(PublishedValue, other.PublishedValue);
}
public override int GetHashCode()
{
var hashCode = -1254204277;
var hashCode = 1885328050;
hashCode = hashCode * -1521134295 + EqualityComparer<string>.Default.GetHashCode(_culture);
hashCode = hashCode * -1521134295 + EqualityComparer<string>.Default.GetHashCode(_segment);
hashCode = hashCode * -1521134295 + EqualityComparer<object>.Default.GetHashCode(EditedValue);
hashCode = hashCode * -1521134295 + EqualityComparer<object>.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<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/Models/ContentTests.cs
+40 -33
View File
@@ -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<ILogger>()) { Alias = "test" };
Composition.Register(_ => new DataEditorCollection(new [] { editor }));
Composition.Register(_ => new DataEditorCollection(new[] { editor }));
Composition.Register<PropertyEditorCollection>();
var dataType = Mock.Of<IDataType>();
Mock.Get(dataType).Setup(x => x.Configuration).Returns(() => new object());
@@ -55,7 +54,7 @@ namespace Umbraco.Tests.Models
var mediaTypeService = Mock.Of<IMediaTypeService>();
var memberTypeService = Mock.Of<IMemberTypeService>();
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
src/Umbraco.Tests/Models/PropertyTests.cs
+63
View File
@@ -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<int>(() => 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));
}
}
}
}
src/Umbraco.Tests/Umbraco.Tests.csproj
+1
View File
@@ -142,6 +142,7 @@
<Compile Include="Models\CultureImpactTests.cs" />
<Compile Include="Models\ImageProcessorImageUrlGeneratorTest.cs" />
<Compile Include="Models\PathValidationTests.cs" />
<Compile Include="Models\PropertyTests.cs" />
<Compile Include="Models\VariationTests.cs" />
<Compile Include="Persistence\Mappers\MapperTestBase.cs" />
<Compile Include="Persistence\Repositories\DocumentRepositoryTest.cs" />