From 1db635f24c43a78f4c359de3288b423cbc2c37db Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 2 Feb 2016 01:32:36 +0100 Subject: [PATCH] Updates DeepCloneableList to support behaviors, for the FullDataSetCachePolicy we only want to clone when writing to cache, not when reading, the cloning will then be done on individual items after filtering by the FullDataSetRepositoryCachePolicy --- .../Cache/FullDataSetRepositoryCachePolicy.cs | 33 +++++---- .../Collections/DeepCloneableList.cs | 71 ++++++++++++++----- .../Collections/ListCloneBehavior.cs | 20 ++++++ src/Umbraco.Core/Umbraco.Core.csproj | 1 + .../DeepCloneRuntimeCacheProviderTests.cs | 2 +- .../Cache/FullDataSetCachePolicyTests.cs | 4 +- .../Collections/DeepCloneableListTests.cs | 40 ++++++++++- 7 files changed, 134 insertions(+), 37 deletions(-) create mode 100644 src/Umbraco.Core/Collections/ListCloneBehavior.cs 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());