From c4046ecb323c8d105860ded450491b94e1415975 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 22 Apr 2014 13:48:51 +1000 Subject: [PATCH 1/2] Fixes 'Clone' methods to ensure a Deep clone is used --- src/Umbraco.Core/Models/ContentType.cs | 24 ++++-------------------- src/Umbraco.Core/Models/PropertyGroup.cs | 13 +++++++------ src/Umbraco.Core/Models/PropertyType.cs | 7 +++++-- 3 files changed, 16 insertions(+), 28 deletions(-) diff --git a/src/Umbraco.Core/Models/ContentType.cs b/src/Umbraco.Core/Models/ContentType.cs index 710efb9e3a..5fb1fa3100 100644 --- a/src/Umbraco.Core/Models/ContentType.cs +++ b/src/Umbraco.Core/Models/ContentType.cs @@ -143,35 +143,19 @@ namespace Umbraco.Core.Models } /// - /// //TODO: REmove this as it's mostly just a shallow clone and not thread safe - /// Creates a clone of the current entity + /// Creates a deep clone of the current entity with its identity/alias and it's property identities reset /// /// public IContentType Clone(string alias) { - var clone = (ContentType)this.MemberwiseClone(); + var clone = (ContentType)DeepClone(); clone.Alias = alias; clone.Key = Guid.Empty; - var propertyGroups = this.PropertyGroups.Select(x => x.Clone()).ToList(); + var propertyGroups = PropertyGroups.Select(x => x.Clone()).ToList(); clone.PropertyGroups = new PropertyGroupCollection(propertyGroups); - clone.PropertyTypes = this.PropertyTypeCollection.Select(x => x.Clone()).ToList(); + clone.PropertyTypes = PropertyTypeCollection.Select(x => x.Clone()).ToList(); clone.ResetIdentity(); clone.ResetDirtyProperties(false); - - foreach (var propertyGroup in clone.PropertyGroups) - { - propertyGroup.ResetIdentity(); - foreach (var propertyType in propertyGroup.PropertyTypes) - { - propertyType.ResetIdentity(); - } - } - - foreach (var propertyType in clone.PropertyTypes.Where(x => x.HasIdentity)) - { - propertyType.ResetIdentity(); - } - return clone; } diff --git a/src/Umbraco.Core/Models/PropertyGroup.cs b/src/Umbraco.Core/Models/PropertyGroup.cs index 64ebab2961..4de4efb4e1 100644 --- a/src/Umbraco.Core/Models/PropertyGroup.cs +++ b/src/Umbraco.Core/Models/PropertyGroup.cs @@ -141,16 +141,17 @@ namespace Umbraco.Core.Models return hashName ^ hashId; } - //TODO: Remove this, its mostly a shallow clone and is not thread safe + /// + /// Creates a deep clone of the current entity with its identity and it's property identities reset + /// + /// internal PropertyGroup Clone() { - var clone = (PropertyGroup)this.MemberwiseClone(); + var clone = (PropertyGroup)DeepClone(); var collection = new PropertyTypeCollection(); - foreach (var propertyType in this.PropertyTypes) + foreach (var propertyType in PropertyTypes) { - var property = propertyType.Clone(); - property.ResetIdentity(); - property.ResetDirtyProperties(false); + var property = propertyType.Clone(); collection.Add(property); } clone.PropertyTypes = collection; diff --git a/src/Umbraco.Core/Models/PropertyType.cs b/src/Umbraco.Core/Models/PropertyType.cs index 5f583f3d8e..e86b6df75e 100644 --- a/src/Umbraco.Core/Models/PropertyType.cs +++ b/src/Umbraco.Core/Models/PropertyType.cs @@ -426,10 +426,13 @@ namespace Umbraco.Core.Models return hashName ^ hashAlias; } - //TODO: Remove this + /// + /// Creates a deep clone of the current entity with its identity and it's property identities reset + /// + /// internal PropertyType Clone() { - var clone = (PropertyType)this.MemberwiseClone(); + var clone = (PropertyType)DeepClone(); clone.ResetIdentity(); clone.ResetDirtyProperties(false); return clone; From f673cb024a5b1edef18a9069adaa727cd374d6e2 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 22 Apr 2014 14:00:03 +1000 Subject: [PATCH 2/2] Adds unit test for ensuring reset identities on cloning a content type --- src/Umbraco.Tests/Models/ContentTypeTests.cs | 67 ++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/src/Umbraco.Tests/Models/ContentTypeTests.cs b/src/Umbraco.Tests/Models/ContentTypeTests.cs index af94f80fd4..0fc1a52a28 100644 --- a/src/Umbraco.Tests/Models/ContentTypeTests.cs +++ b/src/Umbraco.Tests/Models/ContentTypeTests.cs @@ -24,6 +24,69 @@ namespace Umbraco.Tests.Models } + [Test] + public void Can_Deep_Clone_Content_Type_With_Reset_Identities() + { + var contentType = MockedContentTypes.CreateTextpageContentType(); + contentType.Id = 99; + + var i = 200; + foreach (var propertyType in contentType.PropertyTypes) + { + propertyType.Id = ++i; + } + foreach (var group in contentType.PropertyGroups) + { + group.Id = ++i; + } + //add a property type without a property group + contentType.PropertyTypeCollection.Add( + new PropertyType(new Guid(), DataTypeDatabaseType.Ntext) { Alias = "title2", Name = "Title2", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 }); + + contentType.AllowedTemplates = new[] { new Template("-1,2", "Name", "name") { Id = 200 }, new Template("-1,3", "Name2", "name2") { Id = 201 } }; + contentType.AllowedContentTypes = new[] { new ContentTypeSort(new Lazy(() => 888), 8, "sub"), new ContentTypeSort(new Lazy(() => 889), 9, "sub2") }; + contentType.Id = 10; + contentType.CreateDate = DateTime.Now; + contentType.CreatorId = 22; + contentType.SetDefaultTemplate(new Template("-1,2,3,4", "Test Template", "testTemplate") + { + Id = 88 + }); + contentType.Description = "test"; + contentType.Icon = "icon"; + contentType.IsContainer = true; + contentType.Thumbnail = "thumb"; + contentType.Key = Guid.NewGuid(); + contentType.Level = 3; + contentType.Path = "-1,4,10"; + contentType.SortOrder = 5; + contentType.Trashed = false; + contentType.UpdateDate = DateTime.Now; + + //ensure that nothing is marked as dirty + contentType.ResetDirtyProperties(false); + + var clone = (ContentType)contentType.Clone("newAlias"); + + Assert.AreEqual("newAlias", clone.Alias); + Assert.AreNotEqual("newAlias", contentType.Alias); + Assert.IsFalse(clone.HasIdentity); + + foreach (var propertyGroup in clone.PropertyGroups) + { + Assert.IsFalse(propertyGroup.HasIdentity); + foreach (var propertyType in propertyGroup.PropertyTypes) + { + Assert.IsFalse(propertyType.HasIdentity); + } + } + + foreach (var propertyType in clone.PropertyTypes.Where(x => x.HasIdentity)) + { + Assert.IsFalse(propertyType.HasIdentity); + } + } + [Test] public void Can_Deep_Clone_Content_Type() { @@ -36,6 +99,10 @@ namespace Umbraco.Tests.Models { propertyType.Id = ++i; } + foreach (var group in contentType.PropertyGroups) + { + group.Id = ++i; + } contentType.AllowedTemplates = new[] { new Template("-1,2", "Name", "name") { Id = 200 }, new Template("-1,3", "Name2", "name2") { Id = 201 } }; contentType.AllowedContentTypes = new[] {new ContentTypeSort(new Lazy(() => 888), 8, "sub"), new ContentTypeSort(new Lazy(() => 889), 9, "sub2")}; contentType.Id = 10;