diff --git a/src/Umbraco.Core/Cache/FullDataSetRepositoryCachePolicy.cs b/src/Umbraco.Core/Cache/FullDataSetRepositoryCachePolicy.cs index eeb651dc09..cae7bc16e6 100644 --- a/src/Umbraco.Core/Cache/FullDataSetRepositoryCachePolicy.cs +++ b/src/Umbraco.Core/Cache/FullDataSetRepositoryCachePolicy.cs @@ -85,29 +85,35 @@ namespace Umbraco.Core.Cache public override TEntity Get(TId id, Func getFromRepo) { //Force get all with cache - var found = GetAll(new TId[] { }, ids => _getAllFromRepo()); + var found = GetAll(new TId[] { }, ids => _getAllFromRepo().WhereNotNull()); //we don't have anything in cache (this should never happen), just return from the repo - return found == null - ? getFromRepo(id) - : found.FirstOrDefault(x => _getEntityId(x).Equals(id)); + if (found == null) return getFromRepo(id); + var entity = found.FirstOrDefault(x => _getEntityId(x).Equals(id)); + if (entity == null) return null; + + //We must ensure to deep clone each one out manually since the deep clone list only clones one way + return (TEntity)entity.DeepClone(); } public override TEntity Get(TId id) { //Force get all with cache - var found = GetAll(new TId[] { }, ids => _getAllFromRepo()); + var found = GetAll(new TId[] { }, ids => _getAllFromRepo().WhereNotNull()); //we don't have anything in cache (this should never happen), just return null - return found == null - ? null - : found.FirstOrDefault(x => _getEntityId(x).Equals(id)); + if (found == null) return null; + var entity = found.FirstOrDefault(x => _getEntityId(x).Equals(id)); + if (entity == null) return null; + + //We must ensure to deep clone each one out manually since the deep clone list only clones one way + return (TEntity)entity.DeepClone(); } public override bool Exists(TId id, Func getFromRepo) { //Force get all with cache - var found = GetAll(new TId[] {}, ids => _getAllFromRepo()); + var found = GetAll(new TId[] {}, ids => _getAllFromRepo().WhereNotNull()); //we don't have anything in cache (this should never happen), just return from the repo return found == null @@ -123,12 +129,15 @@ namespace Umbraco.Core.Cache //now that the base result has been calculated, they will all be cached. // Now we can just filter by ids if they have been supplied - return ids.Any() + return (ids.Any() ? result.Where(x => ids.Contains(_getEntityId(x))).ToArray() - : result; + : result) + //We must ensure to deep clone each one out manually since the deep clone list only clones one way + .Select(x => (TEntity)x.DeepClone()) + .ToArray(); } - protected TEntity[] PerformGetAll(Func> getFromRepo) + private TEntity[] PerformGetAll(Func> getFromRepo) { var allEntities = GetAllFromCache(); if (allEntities.Any()) diff --git a/src/Umbraco.Core/Collections/DeepCloneableList.cs b/src/Umbraco.Core/Collections/DeepCloneableList.cs index 365bf53b06..5067562aa7 100644 --- a/src/Umbraco.Core/Collections/DeepCloneableList.cs +++ b/src/Umbraco.Core/Collections/DeepCloneableList.cs @@ -14,18 +14,24 @@ namespace Umbraco.Core.Collections /// internal class DeepCloneableList : List, IDeepCloneable, IRememberBeingDirty { - /// - /// Initializes a new instance of the class that is empty and has the default initial capacity. - /// - public DeepCloneableList() + private readonly ListCloneBehavior _listCloneBehavior; + + public DeepCloneableList(ListCloneBehavior listCloneBehavior) { + _listCloneBehavior = listCloneBehavior; + } + + public DeepCloneableList(IEnumerable collection, ListCloneBehavior listCloneBehavior) : base(collection) + { + _listCloneBehavior = listCloneBehavior; } /// - /// Initializes a new instance of the class that contains elements copied from the specified collection and has sufficient capacity to accommodate the number of elements copied. + /// Default behavior is CloneOnce /// - /// The collection whose elements are copied to the new list. is null. - public DeepCloneableList(IEnumerable collection) : base(collection) + /// + public DeepCloneableList(IEnumerable collection) + : this(collection, ListCloneBehavior.CloneOnce) { } @@ -35,20 +41,47 @@ namespace Umbraco.Core.Collections /// public object DeepClone() { - var newList = new DeepCloneableList(); - foreach (var item in this) + switch (_listCloneBehavior) { - var dc = item as IDeepCloneable; - if (dc != null) - { - newList.Add((T) dc.DeepClone()); - } - else - { - newList.Add(item); - } + case ListCloneBehavior.CloneOnce: + //we are cloning once, so create a new list in none mode + // and deep clone all items into it + var newList = new DeepCloneableList(ListCloneBehavior.None); + foreach (var item in this) + { + var dc = item as IDeepCloneable; + if (dc != null) + { + newList.Add((T)dc.DeepClone()); + } + else + { + newList.Add(item); + } + } + return newList; + case ListCloneBehavior.None: + //we are in none mode, so just return a new list with the same items + return new DeepCloneableList(this, ListCloneBehavior.None); + case ListCloneBehavior.Always: + //always clone to new list + var newList2 = new DeepCloneableList(ListCloneBehavior.Always); + foreach (var item in this) + { + var dc = item as IDeepCloneable; + if (dc != null) + { + newList2.Add((T)dc.DeepClone()); + } + else + { + newList2.Add(item); + } + } + return newList2; + default: + throw new ArgumentOutOfRangeException(); } - return newList; } public bool IsDirty() diff --git a/src/Umbraco.Core/Collections/ListCloneBehavior.cs b/src/Umbraco.Core/Collections/ListCloneBehavior.cs new file mode 100644 index 0000000000..4fe935f7ff --- /dev/null +++ b/src/Umbraco.Core/Collections/ListCloneBehavior.cs @@ -0,0 +1,20 @@ +namespace Umbraco.Core.Collections +{ + internal enum ListCloneBehavior + { + /// + /// When set, DeepClone will clone the items one time and the result list behavior will be None + /// + CloneOnce, + + /// + /// When set, DeepClone will not clone any items + /// + None, + + /// + /// When set, DeepClone will always clone all items + /// + Always + } +} \ No newline at end of file diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index e0991e9ba7..49eaa8a629 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -188,6 +188,7 @@ + diff --git a/src/Umbraco.Tests/Cache/DeepCloneRuntimeCacheProviderTests.cs b/src/Umbraco.Tests/Cache/DeepCloneRuntimeCacheProviderTests.cs index 63225f6725..39e5dd2cb1 100644 --- a/src/Umbraco.Tests/Cache/DeepCloneRuntimeCacheProviderTests.cs +++ b/src/Umbraco.Tests/Cache/DeepCloneRuntimeCacheProviderTests.cs @@ -41,7 +41,7 @@ namespace Umbraco.Tests.Cache [Test] public void Clones_List() { - var original = new DeepCloneableList(); + var original = new DeepCloneableList(ListCloneBehavior.Always); original.Add(new DeepCloneableListTests.TestClone()); original.Add(new DeepCloneableListTests.TestClone()); original.Add(new DeepCloneableListTests.TestClone()); diff --git a/src/Umbraco.Tests/Cache/FullDataSetCachePolicyTests.cs b/src/Umbraco.Tests/Cache/FullDataSetCachePolicyTests.cs index d3df319ac7..96e22e3aff 100644 --- a/src/Umbraco.Tests/Cache/FullDataSetCachePolicyTests.cs +++ b/src/Umbraco.Tests/Cache/FullDataSetCachePolicyTests.cs @@ -81,7 +81,7 @@ namespace Umbraco.Tests.Cache cache.Setup(x => x.GetCacheItem(It.IsAny())).Returns(() => { //return null if this is the first pass - return cached.Any() ? new DeepCloneableList() : null; + return cached.Any() ? new DeepCloneableList(ListCloneBehavior.CloneOnce) : null; }); var defaultPolicy = new FullDataSetRepositoryCachePolicy(cache.Object, item => item.Id, () => getAll, false); @@ -144,7 +144,7 @@ namespace Umbraco.Tests.Cache var cache = new Mock(); - cache.Setup(x => x.GetCacheItem(It.IsAny())).Returns(() => new DeepCloneableList + cache.Setup(x => x.GetCacheItem(It.IsAny())).Returns(() => new DeepCloneableList(ListCloneBehavior.CloneOnce) { new AuditItem(1, "blah", AuditType.Copy, 123), new AuditItem(2, "blah2", AuditType.Copy, 123) diff --git a/src/Umbraco.Tests/Collections/DeepCloneableListTests.cs b/src/Umbraco.Tests/Collections/DeepCloneableListTests.cs index fcc50df60c..d478192e02 100644 --- a/src/Umbraco.Tests/Collections/DeepCloneableListTests.cs +++ b/src/Umbraco.Tests/Collections/DeepCloneableListTests.cs @@ -12,10 +12,44 @@ namespace Umbraco.Tests.Collections [TestFixture] public class DeepCloneableListTests { + [Test] + public void Deep_Clones_Each_Item_Once() + { + var list = new DeepCloneableList(ListCloneBehavior.CloneOnce); + list.Add(new TestClone()); + list.Add(new TestClone()); + list.Add(new TestClone()); + + var cloned = list.DeepClone() as DeepCloneableList; + + //Test that each item in the sequence is equal - based on the equality comparer of TestClone (i.e. it's ID) + Assert.IsTrue(list.SequenceEqual(cloned)); + + //Test that each instance in the list is not the same one + foreach (var item in list) + { + var clone = cloned.Single(x => x.Id == item.Id); + Assert.AreNotSame(item, clone); + } + + //clone again from the clone - since it's clone once the items should be the same + var cloned2 = cloned.DeepClone() as DeepCloneableList; + + //Test that each item in the sequence is equal - based on the equality comparer of TestClone (i.e. it's ID) + Assert.IsTrue(cloned.SequenceEqual(cloned2)); + + //Test that each instance in the list is the same one + foreach (var item in cloned) + { + var clone = cloned2.Single(x => x.Id == item.Id); + Assert.AreSame(item, clone); + } + } + [Test] public void Deep_Clones_All_Elements() { - var list = new DeepCloneableList(); + var list = new DeepCloneableList(ListCloneBehavior.Always); list.Add(new TestClone()); list.Add(new TestClone()); list.Add(new TestClone()); @@ -30,7 +64,7 @@ namespace Umbraco.Tests.Collections [Test] public void Clones_Each_Item() { - var list = new DeepCloneableList(); + var list = new DeepCloneableList(ListCloneBehavior.Always); list.Add(new TestClone()); list.Add(new TestClone()); list.Add(new TestClone()); @@ -46,7 +80,7 @@ namespace Umbraco.Tests.Collections [Test] public void Cloned_Sequence_Equals() { - var list = new DeepCloneableList(); + var list = new DeepCloneableList(ListCloneBehavior.Always); list.Add(new TestClone()); list.Add(new TestClone()); list.Add(new TestClone());