diff --git a/src/Umbraco.Core/Models/EntityBase/Entity.cs b/src/Umbraco.Core/Models/EntityBase/Entity.cs index b7d054c676..315e2697c0 100644 --- a/src/Umbraco.Core/Models/EntityBase/Entity.cs +++ b/src/Umbraco.Core/Models/EntityBase/Entity.cs @@ -240,52 +240,6 @@ namespace Umbraco.Core.Models.EntityBase DeepCloneHelper.DeepCloneRefProperties(this, clone); clone.ResetDirtyProperties(false); return clone; - - //Using data contract serializer - has issues - - //var s = Serialize(this); - //var d = Deserialize(s, this.GetType()); - //return d; - - //Using binary serializer - has issues - - //using (var memoryStream = new MemoryStream(10)) - //{ - //IFormatter formatter = new BinaryFormatter(); - //formatter.Serialize(memoryStream, this); - //memoryStream.Seek(0, SeekOrigin.Begin); - //return formatter.Deserialize(memoryStream); - //} } - - // serialize/deserialize with data contracts: - - //public static string Serialize(object obj) - //{ - // using (var memoryStream = new MemoryStream()) - // using (var reader = new StreamReader(memoryStream)) - // { - // var serializer = new DataContractSerializer(obj.GetType()); - // serializer.WriteObject(memoryStream, obj); - // memoryStream.Position = 0; - // return reader.ReadToEnd(); - // } - //} - - //public static object Deserialize(string xml, Type toType) - //{ - // using (Stream stream = new MemoryStream()) - // { - // using (var writer = new StreamWriter(stream, Encoding.UTF8)) - // { - // writer.Write(xml); - // //byte[] data = Encoding.UTF8.GetBytes(xml); - // //stream.Write(data, 0, data.Length); - // stream.Position = 0; - // var deserializer = new DataContractSerializer(toType); - // return deserializer.ReadObject(stream); - // } - // } - //} } } \ No newline at end of file diff --git a/src/Umbraco.Core/Models/IMacroProperty.cs b/src/Umbraco.Core/Models/IMacroProperty.cs index 551d60304c..c39e777a0e 100644 --- a/src/Umbraco.Core/Models/IMacroProperty.cs +++ b/src/Umbraco.Core/Models/IMacroProperty.cs @@ -6,7 +6,7 @@ namespace Umbraco.Core.Models /// /// Defines a Property for a Macro /// - public interface IMacroProperty : IValueObject + public interface IMacroProperty : IValueObject, IDeepCloneable { [DataMember] int Id { get; set; } diff --git a/src/Umbraco.Core/Models/Macro.cs b/src/Umbraco.Core/Models/Macro.cs index b892b8064e..eb8e0b7a66 100644 --- a/src/Umbraco.Core/Models/Macro.cs +++ b/src/Umbraco.Core/Models/Macro.cs @@ -138,12 +138,11 @@ namespace Umbraco.Core.Models var alias = prop.Alias; - //remove from the removed/added props (since people could add/remove all they want in one request) - _removedProperties.RemoveAll(s => s == alias); - _addedProperties.RemoveAll(s => s == alias); - - //add to the added props - _addedProperties.Add(alias); + if (_addedProperties.Contains(alias) == false) + { + //add to the added props + _addedProperties.Add(alias); + } } else if (e.Action == NotifyCollectionChangedAction.Remove) { @@ -153,12 +152,10 @@ namespace Umbraco.Core.Models var alias = prop.Alias; - //remove from the removed/added props (since people could add/remove all they want in one request) - _removedProperties.RemoveAll(s => s == alias); - _addedProperties.RemoveAll(s => s == alias); - - //add to the added props - _removedProperties.Add(alias); + if (_removedProperties.Contains(alias) == false) + { + _removedProperties.Add(alias); + } } } @@ -404,7 +401,8 @@ namespace Umbraco.Core.Models clone._addedProperties = new List(); clone._removedProperties = new List(); - clone._properties = new MacroPropertyCollection(); + clone._properties = (MacroPropertyCollection)Properties.DeepClone(); + //re-assign the event handler clone._properties.CollectionChanged += clone.PropertiesChanged; clone.ResetDirtyProperties(false); diff --git a/src/Umbraco.Core/Models/MacroProperty.cs b/src/Umbraco.Core/Models/MacroProperty.cs index ef0a024973..17fef44a87 100644 --- a/src/Umbraco.Core/Models/MacroProperty.cs +++ b/src/Umbraco.Core/Models/MacroProperty.cs @@ -11,7 +11,7 @@ namespace Umbraco.Core.Models /// [Serializable] [DataContract(IsReference = true)] - public class MacroProperty : TracksChangesEntityBase, IMacroProperty, IRememberBeingDirty + public class MacroProperty : TracksChangesEntityBase, IMacroProperty, IRememberBeingDirty, IDeepCloneable { public MacroProperty() { @@ -176,5 +176,37 @@ namespace Umbraco.Core.Models }, _editorAlias, PropertyTypeSelector); } } + + public object DeepClone() + { + //Memberwise clone on MacroProperty will work since it doesn't have any deep elements + // for any sub class this will work for standard properties as well that aren't complex object's themselves. + var clone = (MacroProperty)MemberwiseClone(); + //Automatically deep clone ref properties that are IDeepCloneable + DeepCloneHelper.DeepCloneRefProperties(this, clone); + clone.ResetDirtyProperties(false); + return clone; + } + + protected bool Equals(MacroProperty other) + { + return string.Equals(_alias, other._alias) && _id == other._id; + } + + public override bool Equals(object obj) + { + if (ReferenceEquals(null, obj)) return false; + if (ReferenceEquals(this, obj)) return true; + if (obj.GetType() != this.GetType()) return false; + return Equals((MacroProperty) obj); + } + + public override int GetHashCode() + { + unchecked + { + return ((_alias != null ? _alias.GetHashCode() : 0)*397) ^ _id; + } + } } } \ No newline at end of file diff --git a/src/Umbraco.Core/Models/MacroPropertyCollection.cs b/src/Umbraco.Core/Models/MacroPropertyCollection.cs index 6d0c762154..7496b582b4 100644 --- a/src/Umbraco.Core/Models/MacroPropertyCollection.cs +++ b/src/Umbraco.Core/Models/MacroPropertyCollection.cs @@ -8,13 +8,22 @@ namespace Umbraco.Core.Models /// /// A macro's property collection /// - public class MacroPropertyCollection : ObservableDictionary + public class MacroPropertyCollection : ObservableDictionary, IDeepCloneable { public MacroPropertyCollection() : base(property => property.Alias) { } + public object DeepClone() + { + var clone = new MacroPropertyCollection(); + foreach (var item in this) + { + clone.Add((IMacroProperty)item.DeepClone()); + } + return clone; + } } } \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/Repositories/MacroRepository.cs b/src/Umbraco.Core/Persistence/Repositories/MacroRepository.cs index 805177be9a..4fa6215477 100644 --- a/src/Umbraco.Core/Persistence/Repositories/MacroRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/MacroRepository.cs @@ -169,6 +169,14 @@ namespace Umbraco.Core.Persistence.Repositories var macro = (Macro)entity; if (macro.IsPropertyDirty("Properties")) { + //now we need to delete any props that have been removed + foreach (var propAlias in macro.RemovedProperties) + { + //delete the property + Database.Delete("WHERE macro=@macroId AND macroPropertyAlias=@propAlias", + new { macroId = macro.Id, propAlias = propAlias }); + } + //for any that exist on the object, we need to determine if we need to update or insert foreach (var propDto in dto.MacroPropertyDtos) { @@ -184,13 +192,7 @@ namespace Umbraco.Core.Persistence.Repositories } } - //now we need to delete any props that have been removed - foreach (var propAlias in macro.RemovedProperties) - { - //delete the property - Database.Delete("WHERE macro=@macroId AND macroPropertyAlias=@propAlias", - new { macroId = macro.Id, propAlias = propAlias }); - } + } ((ICanBeDirty)entity).ResetDirtyProperties(); diff --git a/src/Umbraco.Tests/Models/MacroTests.cs b/src/Umbraco.Tests/Models/MacroTests.cs index 273971c93c..b11814d622 100644 --- a/src/Umbraco.Tests/Models/MacroTests.cs +++ b/src/Umbraco.Tests/Models/MacroTests.cs @@ -1,3 +1,4 @@ +using System.Linq; using NUnit.Framework; using Umbraco.Core.Models; using Umbraco.Core.Models.EntityBase; @@ -19,6 +20,7 @@ namespace Umbraco.Tests.Models public void Can_Deep_Clone() { var macro = new Macro(1, true, 3, "test", "Test", "blah", "blah", "xslt", false, true, true, "script"); + macro.Properties.Add(new MacroProperty(6, "rewq", "REWQ", 1, "asdfasdf")); var clone = (Macro)macro.DeepClone(); @@ -26,6 +28,14 @@ namespace Umbraco.Tests.Models Assert.AreEqual(clone, macro); Assert.AreEqual(clone.Id, macro.Id); + Assert.AreEqual(clone.Properties.Count, macro.Properties.Count); + + for (int i = 0; i < clone.Properties.Count; i++) + { + Assert.AreEqual(clone.Properties[i], macro.Properties[i]); + Assert.AreNotSame(clone.Properties[i], macro.Properties[i]); + } + Assert.AreNotSame(clone.Properties, macro.Properties); Assert.AreNotSame(clone.AddedProperties, macro.AddedProperties); Assert.AreNotSame(clone.RemovedProperties, macro.RemovedProperties); @@ -44,6 +54,9 @@ namespace Umbraco.Tests.Models Assert.IsFalse(asDirty.IsPropertyDirty("Properties")); clone.Properties.Add(new MacroProperty(3, "asdf", "SDF", 3, "asdfasdf")); Assert.IsTrue(asDirty.IsPropertyDirty("Properties")); + Assert.AreEqual(1, clone.AddedProperties.Count()); + clone.Properties.Remove("rewq"); + Assert.AreEqual(1, clone.RemovedProperties.Count()); } diff --git a/src/Umbraco.Tests/Services/MacroServiceTests.cs b/src/Umbraco.Tests/Services/MacroServiceTests.cs index 5ef75d0b14..d88dd26dd4 100644 --- a/src/Umbraco.Tests/Services/MacroServiceTests.cs +++ b/src/Umbraco.Tests/Services/MacroServiceTests.cs @@ -150,6 +150,40 @@ namespace Umbraco.Tests.Services } + [Test] + public void Can_Add_And_Remove_Properties() + { + var macroService = ServiceContext.MacroService; + var macro = new Macro("test", "Test", scriptPath: "~/Views/MacroPartials/Test.cshtml", cacheDuration: 1234); + + //adds some properties + macro.Properties.Add(new MacroProperty("blah1", "Blah1", 0, "blah1")); + macro.Properties.Add(new MacroProperty("blah2", "Blah2", 0, "blah2")); + macro.Properties.Add(new MacroProperty("blah3", "Blah3", 0, "blah3")); + macro.Properties.Add(new MacroProperty("blah4", "Blah4", 0, "blah4")); + macroService.Save(macro); + + var result1 = macroService.GetById(macro.Id); + Assert.AreEqual(4, result1.Properties.Count()); + + //simulate clearing the sections + foreach (var s in result1.Properties.ToArray()) + { + result1.Properties.Remove(s.Alias); + } + //now just re-add a couple + result1.Properties.Add(new MacroProperty("blah3", "Blah3", 0, "blah3")); + result1.Properties.Add(new MacroProperty("blah4", "Blah4", 0, "blah4")); + macroService.Save(result1); + + //assert + + //re-get + result1 = macroService.GetById(result1.Id); + Assert.AreEqual(2, result1.Properties.Count()); + + } + //[Test] //public void Can_Get_Many_By_Alias() //{