diff --git a/src/Umbraco.Core/Cache/RepositoryCachePolicyOptions.cs b/src/Umbraco.Core/Cache/RepositoryCachePolicyOptions.cs index ba7b251aa0..fa334e5c4a 100644 --- a/src/Umbraco.Core/Cache/RepositoryCachePolicyOptions.cs +++ b/src/Umbraco.Core/Cache/RepositoryCachePolicyOptions.cs @@ -11,6 +11,7 @@ public class RepositoryCachePolicyOptions public RepositoryCachePolicyOptions(Func performCount) { PerformCount = performCount; + CacheNullValues = false; GetAllCacheValidateCount = true; GetAllCacheAllowZeroCount = false; } @@ -21,6 +22,7 @@ public class RepositoryCachePolicyOptions public RepositoryCachePolicyOptions() { PerformCount = null; + CacheNullValues = false; GetAllCacheValidateCount = false; GetAllCacheAllowZeroCount = false; } @@ -30,6 +32,11 @@ public class RepositoryCachePolicyOptions /// public Func? PerformCount { get; set; } + /// + /// True if the Get method will cache null results so that the db is not hit for repeated lookups + /// + public bool CacheNullValues { get; set; } + /// /// True/false as to validate the total item count when all items are returned from cache, the default is true but this /// means that a db lookup will occur - though that lookup will probably be significantly less expensive than the diff --git a/src/Umbraco.Infrastructure/Cache/DefaultRepositoryCachePolicy.cs b/src/Umbraco.Infrastructure/Cache/DefaultRepositoryCachePolicy.cs index 7f7f8d6784..9494ed2eea 100644 --- a/src/Umbraco.Infrastructure/Cache/DefaultRepositoryCachePolicy.cs +++ b/src/Umbraco.Infrastructure/Cache/DefaultRepositoryCachePolicy.cs @@ -24,6 +24,8 @@ public class DefaultRepositoryCachePolicy : RepositoryCachePolicyB private static readonly TEntity[] _emptyEntities = new TEntity[0]; // const private readonly RepositoryCachePolicyOptions _options; + private const string NullRepresentationInCache = "*NULL*"; + public DefaultRepositoryCachePolicy(IAppPolicyCache cache, IScopeAccessor scopeAccessor, RepositoryCachePolicyOptions options) : base(cache, scopeAccessor) => _options = options ?? throw new ArgumentNullException(nameof(options)); @@ -116,6 +118,7 @@ public class DefaultRepositoryCachePolicy : RepositoryCachePolicyB { // whatever happens, clear the cache var cacheKey = GetEntityCacheKey(entity.Id); + Cache.Clear(cacheKey); // if there's a GetAllCacheAllowZeroCount cache, ensure it is cleared @@ -127,20 +130,36 @@ public class DefaultRepositoryCachePolicy : RepositoryCachePolicyB public override TEntity? Get(TId? id, Func performGet, Func?> performGetAll) { var cacheKey = GetEntityCacheKey(id); + TEntity? fromCache = Cache.GetCacheItem(cacheKey); - // if found in cache then return else fetch and cache - if (fromCache != null) + // If found in cache then return immediately. + if (fromCache is not null) { return fromCache; } + // Because TEntity can never be a string, we will never be in a position where the proxy value collides withs a real value. + // Therefore this point can only be reached if there is a proxy null value => becomes null when cast to TEntity above OR the item simply does not exist. + // If we've cached a "null" value, return null. + if (_options.CacheNullValues && Cache.GetCacheItem(cacheKey) == NullRepresentationInCache) + { + return null; + } + + // Otherwise go to the database to retrieve. TEntity? entity = performGet(id); if (entity != null && entity.HasIdentity) { + // If we've found an identified entity, cache it for subsequent retrieval. InsertEntity(cacheKey, entity); } + else if (entity is null && _options.CacheNullValues) + { + // If we've not found an entity, and we're caching null values, cache a "null" value. + InsertNull(cacheKey); + } return entity; } @@ -248,6 +267,15 @@ public class DefaultRepositoryCachePolicy : RepositoryCachePolicyB protected virtual void InsertEntity(string cacheKey, TEntity entity) => Cache.Insert(cacheKey, () => entity, TimeSpan.FromMinutes(5), true); + protected virtual void InsertNull(string cacheKey) + { + // We can't actually cache a null value, as in doing so wouldn't be able to distinguish between + // a value that does exist but isn't yet cached, or a value that has been explicitly cached with a null value. + // Both would return null when we retrieve from the cache and we couldn't distinguish between the two. + // So we cache a special value that represents null, and then we can check for that value when we retrieve from the cache. + Cache.Insert(cacheKey, () => NullRepresentationInCache, TimeSpan.FromMinutes(5), true); + } + protected virtual void InsertEntities(TId[]? ids, TEntity[]? entities) { if (ids?.Length == 0 && entities?.Length == 0 && _options.GetAllCacheAllowZeroCount) diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DictionaryRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DictionaryRepository.cs index 909c9cfec2..bf4799e938 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DictionaryRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DictionaryRepository.cs @@ -102,11 +102,10 @@ internal class DictionaryRepository : EntityRepositoryBase var options = new RepositoryCachePolicyOptions { // allow zero to be cached - GetAllCacheAllowZeroCount = true, + GetAllCacheAllowZeroCount = true }; - return new SingleItemsOnlyRepositoryCachePolicy(GlobalIsolatedCache, ScopeAccessor, - options); + return new SingleItemsOnlyRepositoryCachePolicy(GlobalIsolatedCache, ScopeAccessor, options); } protected IDictionaryItem ConvertFromDto(DictionaryDto dto) @@ -190,11 +189,10 @@ internal class DictionaryRepository : EntityRepositoryBase var options = new RepositoryCachePolicyOptions { // allow zero to be cached - GetAllCacheAllowZeroCount = true, + GetAllCacheAllowZeroCount = true }; - return new SingleItemsOnlyRepositoryCachePolicy(GlobalIsolatedCache, ScopeAccessor, - options); + return new SingleItemsOnlyRepositoryCachePolicy(GlobalIsolatedCache, ScopeAccessor, options); } } @@ -228,12 +226,13 @@ internal class DictionaryRepository : EntityRepositoryBase { var options = new RepositoryCachePolicyOptions { + // allow null to be cached + CacheNullValues = true, // allow zero to be cached - GetAllCacheAllowZeroCount = true, + GetAllCacheAllowZeroCount = true }; - return new SingleItemsOnlyRepositoryCachePolicy(GlobalIsolatedCache, ScopeAccessor, - options); + return new SingleItemsOnlyRepositoryCachePolicy(GlobalIsolatedCache, ScopeAccessor, options); } }