diff --git a/src/Umbraco.Core/Cache/HttpRuntimeCacheProvider.cs b/src/Umbraco.Core/Cache/HttpRuntimeCacheProvider.cs index dcd5198eec..88fee8ef3a 100644 --- a/src/Umbraco.Core/Cache/HttpRuntimeCacheProvider.cs +++ b/src/Umbraco.Core/Cache/HttpRuntimeCacheProvider.cs @@ -140,6 +140,7 @@ namespace Umbraco.Core.Cache var sliding = isSliding == false ? System.Web.Caching.Cache.NoSlidingExpiration : (timeout ?? System.Web.Caching.Cache.NoSlidingExpiration); lck.UpgradeToWriteLock(); + //NOTE: 'Insert' on System.Web.Caching.Cache actually does an add or update! _cache.Insert(cacheKey, result, dependency, absolute, sliding, priority, removedCallback); } } @@ -197,6 +198,7 @@ namespace Umbraco.Core.Cache using (new WriteLock(_locker)) { + //NOTE: 'Insert' on System.Web.Caching.Cache actually does an add or update! _cache.Insert(cacheKey, result, dependency, absolute, sliding, priority, removedCallback); } } diff --git a/src/Umbraco.Core/Cache/ObjectCacheRuntimeCacheProvider.cs b/src/Umbraco.Core/Cache/ObjectCacheRuntimeCacheProvider.cs index dfe00449fc..76519a6722 100644 --- a/src/Umbraco.Core/Cache/ObjectCacheRuntimeCacheProvider.cs +++ b/src/Umbraco.Core/Cache/ObjectCacheRuntimeCacheProvider.cs @@ -216,6 +216,7 @@ namespace Umbraco.Core.Cache var policy = GetPolicy(timeout, isSliding, removedCallback, dependentFiles); lck.UpgradeToWriteLock(); + //NOTE: This does an add or update MemoryCache.Set(cacheKey, result, policy); } } @@ -242,6 +243,7 @@ namespace Umbraco.Core.Cache if (value == null) return; // do not store null values (backward compat) var policy = GetPolicy(timeout, isSliding, removedCallback, dependentFiles); + //NOTE: This does an add or update MemoryCache.Set(cacheKey, result, policy); } diff --git a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs index beb442ae86..8069cb5adc 100644 --- a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs @@ -658,7 +658,7 @@ namespace Umbraco.Core.Persistence.Repositories // then we can use that entity. Otherwise if it is not published (which can be the case // because we only store the 'latest' entries in the cache which might not be the published // version) - var fromCache = RepositoryCache.RuntimeCache.GetCacheItem(GetCacheIdKey(dto.NodeId)); + var fromCache = RuntimeCache.GetCacheItem(GetCacheIdKey(dto.NodeId)); //var fromCache = TryGetFromCache(dto.NodeId); if (fromCache != null && fromCache.Published) { diff --git a/src/Umbraco.Core/Persistence/Repositories/DeepCloneRuntimeCacheProvider.cs b/src/Umbraco.Core/Persistence/Repositories/DeepCloneRuntimeCacheProvider.cs index fa998f06a2..a87ec1e764 100644 --- a/src/Umbraco.Core/Persistence/Repositories/DeepCloneRuntimeCacheProvider.cs +++ b/src/Umbraco.Core/Persistence/Repositories/DeepCloneRuntimeCacheProvider.cs @@ -9,118 +9,128 @@ using Umbraco.Core.Models.EntityBase; namespace Umbraco.Core.Persistence.Repositories { /// - /// Ensures that all inserts and returns are a deep cloned copy of the item when - /// the item is IDeepCloneable + /// A wrapper for any IRuntimeCacheProvider that ensures that all inserts and returns + /// are a deep cloned copy of the item when the item is IDeepCloneable and that tracks changes are + /// reset if the object is TracksChangesEntityBase /// internal class DeepCloneRuntimeCacheProvider : IRuntimeCacheProvider { - private readonly IRuntimeCacheProvider _innerProvider; + internal IRuntimeCacheProvider InnerProvider { get; private set; } public DeepCloneRuntimeCacheProvider(IRuntimeCacheProvider innerProvider) { - _innerProvider = innerProvider; + InnerProvider = innerProvider; } #region Clear - doesn't require any changes public void ClearAllCache() { - _innerProvider.ClearAllCache(); + InnerProvider.ClearAllCache(); } public void ClearCacheItem(string key) { - _innerProvider.ClearCacheItem(key); + InnerProvider.ClearCacheItem(key); } public void ClearCacheObjectTypes(string typeName) { - _innerProvider.ClearCacheObjectTypes(typeName); + InnerProvider.ClearCacheObjectTypes(typeName); } public void ClearCacheObjectTypes() { - _innerProvider.ClearCacheObjectTypes(); + InnerProvider.ClearCacheObjectTypes(); } public void ClearCacheObjectTypes(Func predicate) { - _innerProvider.ClearCacheObjectTypes(predicate); + InnerProvider.ClearCacheObjectTypes(predicate); } public void ClearCacheByKeySearch(string keyStartsWith) { - _innerProvider.ClearCacheByKeySearch(keyStartsWith); + InnerProvider.ClearCacheByKeySearch(keyStartsWith); } public void ClearCacheByKeyExpression(string regexString) { - _innerProvider.ClearCacheByKeyExpression(regexString); + InnerProvider.ClearCacheByKeyExpression(regexString); } #endregion public IEnumerable GetCacheItemsByKeySearch(string keyStartsWith) { - return _innerProvider.GetCacheItemsByKeySearch(keyStartsWith) + return InnerProvider.GetCacheItemsByKeySearch(keyStartsWith) .Select(CheckCloneableAndTracksChanges); } public IEnumerable GetCacheItemsByKeyExpression(string regexString) { - return _innerProvider.GetCacheItemsByKeyExpression(regexString) + return InnerProvider.GetCacheItemsByKeyExpression(regexString) .Select(CheckCloneableAndTracksChanges); } public object GetCacheItem(string cacheKey) { - var item = _innerProvider.GetCacheItem(cacheKey); + var item = InnerProvider.GetCacheItem(cacheKey); return CheckCloneableAndTracksChanges(item); } public object GetCacheItem(string cacheKey, Func getCacheItem) { - return _innerProvider.GetCacheItem(cacheKey, () => + return InnerProvider.GetCacheItem(cacheKey, () => { - //Resolve the item but returned the cloned/reset item - var item = getCacheItem(); - return CheckCloneableAndTracksChanges(item); + var result = DictionaryCacheProviderBase.GetSafeLazy(getCacheItem); + var value = result.Value; // force evaluation now - this may throw if cacheItem throws, and then nothing goes into cache + if (value == null) return null; // do not store null values (backward compat) + + return CheckCloneableAndTracksChanges(value); }); } public object GetCacheItem(string cacheKey, Func getCacheItem, TimeSpan? timeout, bool isSliding = false, CacheItemPriority priority = CacheItemPriority.Normal, CacheItemRemovedCallback removedCallback = null, string[] dependentFiles = null) { - return _innerProvider.GetCacheItem(cacheKey, () => + return InnerProvider.GetCacheItem(cacheKey, () => { - //Resolve the item but returned the cloned/reset item - var item = getCacheItem(); - return CheckCloneableAndTracksChanges(item); + var result = DictionaryCacheProviderBase.GetSafeLazy(getCacheItem); + var value = result.Value; // force evaluation now - this may throw if cacheItem throws, and then nothing goes into cache + if (value == null) return null; // do not store null values (backward compat) + + return CheckCloneableAndTracksChanges(value); }, timeout, isSliding, priority, removedCallback, dependentFiles); } public void InsertCacheItem(string cacheKey, Func getCacheItem, TimeSpan? timeout = null, bool isSliding = false, CacheItemPriority priority = CacheItemPriority.Normal, CacheItemRemovedCallback removedCallback = null, string[] dependentFiles = null) { - _innerProvider.InsertCacheItem(cacheKey, () => + InnerProvider.InsertCacheItem(cacheKey, () => { - //Resolve the item but returned the cloned/reset item - var item = getCacheItem(); - return CheckCloneableAndTracksChanges(item); + var result = DictionaryCacheProviderBase.GetSafeLazy(getCacheItem); + var value = result.Value; // force evaluation now - this may throw if cacheItem throws, and then nothing goes into cache + if (value == null) return null; // do not store null values (backward compat) + + return CheckCloneableAndTracksChanges(value); }, timeout, isSliding, priority, removedCallback, dependentFiles); } private static object CheckCloneableAndTracksChanges(object input) { - var entity = input as IDeepCloneable; - if (entity == null) return input; + var cloneable = input as IDeepCloneable; + if (cloneable != null) + { + input = cloneable.DeepClone(); + } - var cloned = entity.DeepClone(); //on initial construction we don't want to have dirty properties tracked // http://issues.umbraco.org/issue/U4-1946 - var tracksChanges = cloned as TracksChangesEntityBase; + var tracksChanges = input as IRememberBeingDirty; if (tracksChanges != null) { tracksChanges.ResetDirtyProperties(false); - return tracksChanges; + input = tracksChanges; } - return cloned; + + return input; } } } \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/Repositories/DictionaryRepository.cs b/src/Umbraco.Core/Persistence/Repositories/DictionaryRepository.cs index a8dac969d3..cf3193943b 100644 --- a/src/Umbraco.Core/Persistence/Repositories/DictionaryRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/DictionaryRepository.cs @@ -169,8 +169,8 @@ namespace Umbraco.Core.Persistence.Repositories entity.ResetDirtyProperties(); //Clear the cache entries that exist by uniqueid/item key - RepositoryCache.RuntimeCache.ClearCacheItem(GetCacheIdKey(entity.ItemKey)); - RepositoryCache.RuntimeCache.ClearCacheItem(GetCacheIdKey(entity.Key)); + RuntimeCache.ClearCacheItem(GetCacheIdKey(entity.ItemKey)); + RuntimeCache.ClearCacheItem(GetCacheIdKey(entity.Key)); } protected override void PersistDeletedItem(IDictionaryItem entity) @@ -181,8 +181,8 @@ namespace Umbraco.Core.Persistence.Repositories Database.Delete("WHERE id = @Id", new { Id = entity.Key }); //Clear the cache entries that exist by uniqueid/item key - RepositoryCache.RuntimeCache.ClearCacheItem(GetCacheIdKey(entity.ItemKey)); - RepositoryCache.RuntimeCache.ClearCacheItem(GetCacheIdKey(entity.Key)); + RuntimeCache.ClearCacheItem(GetCacheIdKey(entity.ItemKey)); + RuntimeCache.ClearCacheItem(GetCacheIdKey(entity.Key)); } private void RecursiveDelete(Guid parentId) @@ -196,8 +196,8 @@ namespace Umbraco.Core.Persistence.Repositories Database.Delete("WHERE id = @Id", new { Id = dto.UniqueId }); //Clear the cache entries that exist by uniqueid/item key - RepositoryCache.RuntimeCache.ClearCacheItem(GetCacheIdKey(dto.Key)); - RepositoryCache.RuntimeCache.ClearCacheItem(GetCacheIdKey(dto.UniqueId)); + RuntimeCache.ClearCacheItem(GetCacheIdKey(dto.Key)); + RuntimeCache.ClearCacheItem(GetCacheIdKey(dto.UniqueId)); } } diff --git a/src/Umbraco.Core/Persistence/Repositories/LanguageRepository.cs b/src/Umbraco.Core/Persistence/Repositories/LanguageRepository.cs index 1403d5e6c4..9a789ac3f6 100644 --- a/src/Umbraco.Core/Persistence/Repositories/LanguageRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/LanguageRepository.cs @@ -131,8 +131,8 @@ namespace Umbraco.Core.Persistence.Repositories entity.ResetDirtyProperties(); //Clear the cache entries that exist by key/iso - RepositoryCache.RuntimeCache.ClearCacheItem(GetCacheIdKey(entity.IsoCode)); - RepositoryCache.RuntimeCache.ClearCacheItem(GetCacheIdKey(entity.CultureName)); + RuntimeCache.ClearCacheItem(GetCacheIdKey(entity.IsoCode)); + RuntimeCache.ClearCacheItem(GetCacheIdKey(entity.CultureName)); } protected override void PersistDeletedItem(ILanguage entity) @@ -140,8 +140,8 @@ namespace Umbraco.Core.Persistence.Repositories base.PersistDeletedItem(entity); //Clear the cache entries that exist by key/iso - RepositoryCache.RuntimeCache.ClearCacheItem(GetCacheIdKey(entity.IsoCode)); - RepositoryCache.RuntimeCache.ClearCacheItem(GetCacheIdKey(entity.CultureName)); + RuntimeCache.ClearCacheItem(GetCacheIdKey(entity.IsoCode)); + RuntimeCache.ClearCacheItem(GetCacheIdKey(entity.CultureName)); } #endregion diff --git a/src/Umbraco.Core/Persistence/Repositories/RepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/RepositoryBase.cs index 887766ddc0..f9088e1911 100644 --- a/src/Umbraco.Core/Persistence/Repositories/RepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/RepositoryBase.cs @@ -26,8 +26,7 @@ namespace Umbraco.Core.Persistence.Repositories //IMPORTANT: We will force the DeepCloneRuntimeCacheProvider to be used here which is a wrapper for the underlying // runtime cache to ensure that anything that can be deep cloned in/out is done so, this also ensures that our tracks // changes entities are reset. - //_cache = new CacheHelper(new DeepCloneRuntimeCacheProvider(cache.RuntimeCache), cache.StaticCache, cache.RequestCache); - _cache = cache; + _cache = new CacheHelper(new DeepCloneRuntimeCacheProvider(cache.RuntimeCache), cache.StaticCache, cache.RequestCache); } /// @@ -126,20 +125,17 @@ namespace Umbraco.Core.Persistence.Repositories /// public TEntity Get(TId id) { - return RuntimeCache.GetCacheItem( - GetCacheIdKey(id), () => - { - var entity = PerformGet(id); - if (entity == null) return null; - //on initial construction we don't want to have dirty properties tracked - // http://issues.umbraco.org/issue/U4-1946 - var asEntity = entity as TracksChangesEntityBase; - if (asEntity != null) - { - asEntity.ResetDirtyProperties(false); - } - return entity; - }); + var cacheKey = GetCacheIdKey(id); + var fromCache = RuntimeCache.GetCacheItem(cacheKey); + + if (fromCache != null) return fromCache; + + var entity = PerformGet(id); + if (entity == null) return null; + + RuntimeCache.InsertCacheItem(cacheKey, () => entity); + + return entity; } protected abstract IEnumerable PerformGetAll(params TId[] ids); diff --git a/src/Umbraco.Tests/Cache/DeepCloneRuntimeCacheProviderTests.cs b/src/Umbraco.Tests/Cache/DeepCloneRuntimeCacheProviderTests.cs new file mode 100644 index 0000000000..44deb1da40 --- /dev/null +++ b/src/Umbraco.Tests/Cache/DeepCloneRuntimeCacheProviderTests.cs @@ -0,0 +1,112 @@ +using System; +using System.Reflection; +using System.Web; +using NUnit.Framework; +using Umbraco.Core; +using Umbraco.Core.Cache; +using Umbraco.Core.Models; +using Umbraco.Core.Models.EntityBase; +using Umbraco.Core.Persistence.Repositories; + +namespace Umbraco.Tests.Cache +{ + [TestFixture] + public class DeepCloneRuntimeCacheProviderTests : RuntimeCacheProviderTests + { + private DeepCloneRuntimeCacheProvider _provider; + + protected override int GetTotalItemCount + { + get { return HttpRuntime.Cache.Count; } + } + + public override void Setup() + { + base.Setup(); + _provider = new DeepCloneRuntimeCacheProvider(new HttpRuntimeCacheProvider(HttpRuntime.Cache)); + } + + internal override ICacheProvider Provider + { + get { return _provider; } + } + + internal override IRuntimeCacheProvider RuntimeProvider + { + get { return _provider; } + } + + [Test] + public void Ensures_Cloned_And_Reset() + { + var original = new TestClass() + { + Name = "hello" + }; + Assert.IsTrue(original.IsDirty()); + + var val = _provider.GetCacheItem("test", () => original); + + Assert.AreNotEqual(original.CloneId, val.CloneId); + Assert.IsFalse(val.IsDirty()); + } + + [Test] + public void DoesNotCacheExceptions() + { + string value; + Assert.Throws(() => { value = (string)_provider.GetCacheItem("key", () => GetValue(1)); }); + Assert.Throws(() => { value = (string)_provider.GetCacheItem("key", () => GetValue(2)); }); + + // does not throw + value = (string)_provider.GetCacheItem("key", () => GetValue(3)); + Assert.AreEqual("succ3", value); + + // cache + value = (string)_provider.GetCacheItem("key", () => GetValue(4)); + Assert.AreEqual("succ3", value); + } + + private static string GetValue(int i) + { + Console.WriteLine("get" + i); + if (i < 3) + throw new Exception("fail"); + return "succ" + i; + } + + private class TestClass : TracksChangesEntityBase, IDeepCloneable + { + public TestClass() + { + CloneId = Guid.NewGuid(); + } + + private static readonly PropertyInfo WriterSelector = ExpressionHelper.GetPropertyInfo(x => x.Name); + + private string _name; + public string Name + { + get { return _name; } + set + { + SetPropertyValueAndDetectChanges(o => + { + _name = value; + return _name; + }, _name, WriterSelector); + } + } + + public Guid CloneId { get; set; } + + public object DeepClone() + { + var shallowClone = (TestClass)MemberwiseClone(); + DeepCloneHelper.DeepCloneRefProperties(this, shallowClone); + shallowClone.CloneId = Guid.NewGuid(); + return shallowClone; + } + } + } +} \ No newline at end of file diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index 9190056d66..ee89db9684 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -171,6 +171,7 @@ +