Skip to content
12 changes: 12 additions & 0 deletions src/Umbraco.Core/Persistence/Repositories/RepositoryCacheKeys.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ public static class RepositoryCacheKeys
/// </summary>
private static readonly ConcurrentDictionary<Type, string> _keys = new();

private static readonly ConcurrentDictionary<Type, string> _guidKeys = new();

/// <summary>
/// Gets the repository cache key for the provided type.
/// </summary>
Expand All @@ -20,6 +22,16 @@ public static class RepositoryCacheKeys
public static string GetKey<T>()
=> _keys.GetOrAdd(typeof(T), static type => "uRepo_" + type.Name + "_");

/// <summary>
/// Gets the GUID-specific repository cache key for the provided type.
/// Uses a distinct prefix so that GUID-keyed entries don't interfere with
/// the int-keyed repository's prefix-based search and count validation.
/// </summary>
/// <typeparam name="T">The entity type to get the cache key for.</typeparam>
/// <returns>A cache key string in the format "uRepoGuid_{TypeName}_".</returns>
public static string GetGuidKey<T>()
=> _guidKeys.GetOrAdd(typeof(T), static type => "uRepoGuid_" + type.Name + "_");

/// <summary>
/// Gets the repository cache key for the provided type and Id.
/// </summary>
Expand Down
131 changes: 131 additions & 0 deletions src/Umbraco.Infrastructure/Cache/GuidReadRepositoryCachePolicy.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
// Copyright (c) Umbraco.
// See LICENSE for more details.

using Umbraco.Cms.Core.Models.Entities;
using Umbraco.Cms.Core.Persistence.Repositories;
using Umbraco.Cms.Core.Scoping;
using Umbraco.Cms.Infrastructure.Scoping;
using Umbraco.Extensions;

namespace Umbraco.Cms.Core.Cache;

/// <summary>
/// A cache policy for GUID-keyed read repositories that share an isolated cache
/// with their parent int-keyed repository.
/// </summary>
/// <typeparam name="TEntity">The type of the entity.</typeparam>
/// <remarks>
/// <para>
/// GUID-keyed read repositories and their parent int-keyed repositories both resolve
/// to the same <see cref="IAppPolicyCache" /> via <c>IsolatedCaches.GetOrCreate&lt;TEntity&gt;()</c>.
/// If both used <see cref="DefaultRepositoryCachePolicy{TEntity, TId}" />, they would share
/// the same cache key prefix (<c>"uRepo_{TypeName}_"</c>), causing the int-keyed repository's
/// prefix-based count validation to always fail (finding 2× the expected entries).
/// </para>
/// <para>
/// This policy uses a separate prefix (<c>"uRepoGuid_{TypeName}_"</c>) so that GUID-keyed
/// entries don't interfere with the int-keyed repository's cache operations.
/// </para>
/// <para>
/// For <see cref="GetAll" /> without specific IDs, this policy always delegates to the
/// repository rather than caching the full set, since the parent int-keyed repository
/// already handles full-set caching with proper count validation.
/// </para>
/// </remarks>
internal sealed class GuidReadRepositoryCachePolicy<TEntity> : RepositoryCachePolicyBase<TEntity, Guid>
where TEntity : class, IEntity
{
internal static string GuidCacheKeyPrefix { get; } = RepositoryCacheKeys.GetGuidKey<TEntity>();

public GuidReadRepositoryCachePolicy(
IAppPolicyCache cache,
IScopeAccessor scopeAccessor,
IRepositoryCacheVersionService repositoryCacheVersionService,
ICacheSyncService cacheSyncService)
: base(cache, scopeAccessor, repositoryCacheVersionService, cacheSyncService)
{
}

/// <inheritdoc />
public override TEntity? Get(Guid id, Func<Guid, TEntity?> performGet, Func<Guid[]?, IEnumerable<TEntity>?> performGetAll)
{
EnsureCacheIsSynced();

var cacheKey = GuidCacheKeyPrefix + id;
TEntity? fromCache = Cache.GetCacheItem<TEntity>(cacheKey);

if (fromCache is not null)
{
return fromCache;
}

TEntity? entity = performGet(id);

if (entity is { HasIdentity: true })
{
Cache.Insert(cacheKey, () => entity, TimeSpan.FromMinutes(5), true);
}

return entity;
}

/// <inheritdoc />
public override TEntity? GetCached(Guid id)
{
EnsureCacheIsSynced();
return Cache.GetCacheItem<TEntity>(GuidCacheKeyPrefix + id);
}

/// <inheritdoc />
public override bool Exists(Guid id, Func<Guid, bool> performExists, Func<Guid[], IEnumerable<TEntity>?> performGetAll)
{
EnsureCacheIsSynced();

TEntity? fromCache = Cache.GetCacheItem<TEntity>(GuidCacheKeyPrefix + id);
return fromCache is not null || performExists(id);
}

/// <inheritdoc />
public override TEntity[] GetAll(Guid[]? ids, Func<Guid[]?, IEnumerable<TEntity>?> performGetAll)
{
EnsureCacheIsSynced();

// For specific IDs, try cache first.
if (ids?.Length > 0)
{
TEntity[] cached = ids
.Select(id => Cache.GetCacheItem<TEntity>(GuidCacheKeyPrefix + id))
.WhereNotNull()
.ToArray();

if (cached.Length == ids.Length)
{
return cached;
}
}

// For the full set (no IDs), always delegate to the repository.
// The parent int-keyed repository handles full-set caching with count validation.
return performGetAll(ids)?.WhereNotNull().ToArray() ?? Array.Empty<TEntity>();
Comment thread
AndyButland marked this conversation as resolved.
Outdated
}

/// <inheritdoc />
public override void Create(TEntity entity, Action<TEntity> persistNew)
=> throw new InvalidOperationException("This method won't be implemented.");

/// <inheritdoc />
public override void Update(TEntity entity, Action<TEntity> persistUpdated)
=> throw new InvalidOperationException("This method won't be implemented.");

/// <inheritdoc />
public override void Delete(TEntity entity, Action<TEntity> persistDeleted)
=> throw new InvalidOperationException("This method won't be implemented.");

/// <inheritdoc />
public override void ClearAll() => Cache.ClearByKey(GuidCacheKeyPrefix);

/// <summary>
/// Gets the GUID-prefixed cache key for the given entity key.
/// </summary>
internal static string GetCacheKey(Guid key) => GuidCacheKeyPrefix + key;
}
Original file line number Diff line number Diff line change
Expand Up @@ -571,6 +571,16 @@ public DataTypeByGuidReadRepository(
cacheSyncService) =>
_outerRepo = outerRepo;

// Use a GUID-specific cache policy with a distinct prefix ("uRepoGuid_IDataType_")
// so that GUID-keyed cache entries don't interfere with the parent int-keyed repository's
// prefix-based search and count validation in DefaultRepositoryCachePolicy.
protected override IRepositoryCachePolicy<IDataType, Guid> CreateCachePolicy()
=> new GuidReadRepositoryCachePolicy<IDataType>(
GlobalIsolatedCache,
ScopeAccessor,
RepositoryCacheVersionService,
CacheSyncService);

protected override IDataType? PerformGet(Guid id)
{
Sql<ISqlContext> sql = _outerRepo.GetBaseQuery(false)
Expand Down Expand Up @@ -675,7 +685,7 @@ public void ClearCacheByKey(Guid key)
IsolatedCache.Clear(cacheKey);
}

private static string GetCacheKey(Guid key) => RepositoryCacheKeys.GetKey<IDataType>() + key;
private static string GetCacheKey(Guid key) => GuidReadRepositoryCachePolicy<IDataType>.GetCacheKey(key);
}

#endregion
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1731,6 +1731,16 @@ public ContentByGuidReadRepository(
cacheSyncService) =>
_outerRepo = outerRepo;

// Use a GUID-specific cache policy with a distinct prefix ("uRepoGuid_IContent_")
// so that GUID-keyed cache entries don't interfere with the parent int-keyed repository's
// prefix-based search and count validation in DefaultRepositoryCachePolicy.
protected override IRepositoryCachePolicy<IContent, Guid> CreateCachePolicy()
=> new GuidReadRepositoryCachePolicy<IContent>(
GlobalIsolatedCache,
ScopeAccessor,
RepositoryCacheVersionService,
CacheSyncService);

protected override IContent? PerformGet(Guid id)
{
Sql<ISqlContext> sql = _outerRepo.GetBaseQuery(QueryType.Single)
Expand Down Expand Up @@ -1811,7 +1821,7 @@ public void PopulateCacheByKey(IEnumerable<IContent> entities)
}
}

private static string GetCacheKey(Guid key) => RepositoryCacheKeys.GetKey<IContent>() + key;
private static string GetCacheKey(Guid key) => GuidReadRepositoryCachePolicy<IContent>.GetCacheKey(key);
}

#endregion
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -669,6 +669,16 @@ public MediaByGuidReadRepository(
cacheSyncService) =>
_outerRepo = outerRepo;

// Use a GUID-specific cache policy with a distinct prefix ("uRepoGuid_IMedia_")
// so that GUID-keyed cache entries don't interfere with the parent int-keyed repository's
// prefix-based search and count validation in DefaultRepositoryCachePolicy.
protected override IRepositoryCachePolicy<IMedia, Guid> CreateCachePolicy()
=> new GuidReadRepositoryCachePolicy<IMedia>(
GlobalIsolatedCache,
ScopeAccessor,
RepositoryCacheVersionService,
CacheSyncService);

protected override IMedia? PerformGet(Guid id)
{
Sql<ISqlContext> sql = _outerRepo.GetBaseQuery(QueryType.Single)
Expand Down Expand Up @@ -749,7 +759,7 @@ public void PopulateCacheByKey(IEnumerable<IMedia> entities)
}
}

private static string GetCacheKey(Guid key) => RepositoryCacheKeys.GetKey<IMedia>() + key;
private static string GetCacheKey(Guid key) => GuidReadRepositoryCachePolicy<IMedia>.GetCacheKey(key);
}

#endregion
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Text;

Check warning on line 1 in src/Umbraco.Infrastructure/Persistence/Repositories/Implement/TemplateRepository.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (main)

❌ New issue: Low Cohesion

This module has at least 3 different responsibilities amongst its 44 functions, threshold = 3. Cohesion is calculated using the LCOM4 metric. Low cohesion means that the module/class has multiple unrelated responsibilities, doing too many things and breaking the Single Responsibility Principle.

Check notice on line 1 in src/Umbraco.Infrastructure/Persistence/Repositories/Implement/TemplateRepository.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (main)

✅ Getting better: Primitive Obsession

The ratio of primitive types in function arguments decreases from 32.89% to 31.94%, threshold = 30.0%. The functions in this file have too many primitive types (e.g. int, double, float) in their function argument lists. Using many primitive types lead to the code smell Primitive Obsession. Avoid adding more primitive arguments.
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using NPoco;
Expand Down Expand Up @@ -99,18 +99,13 @@

// Force population of the full dataset cache so subsequent lookups don't hit the database.
// TemplateRepository uses FullDataSetRepositoryCachePolicy which caches all templates together.
// The GUID read repository delegates to this cache, so no separate GUID caching is needed.
GetMany();

// Also populate the GUID cache so subsequent lookups by GUID don't hit the database.
_templateByGuidReadRepository.PopulateCacheByKey(entity);
}

public override void Delete(ITemplate entity)
{
base.Delete(entity);

// Also clear the GUID cache so subsequent lookups by GUID don't return stale data.
_templateByGuidReadRepository.ClearCacheByKey(entity.Key);
}

public Stream GetFileContentStream(string filepath)
Expand Down Expand Up @@ -378,12 +373,6 @@
//use the underlying GetAll which will force cache all templates
ITemplate? template = GetMany().FirstOrDefault(x => x.Id == id);

if (template != null)
{
// Also populate the GUID cache so subsequent lookups by GUID don't hit the database.
_templateByGuidReadRepository.PopulateCacheByKey(template);
}

return template;
}

Expand Down Expand Up @@ -416,9 +405,6 @@

ITemplate[] templates = dtos.Select(d => MapFromDto(d, childIds)).ToArray();

// Also populate the GUID cache so subsequent lookups by GUID don't hit the database.
_templateByGuidReadRepository.PopulateCacheByKey(templates);

return templates;
}

Expand Down Expand Up @@ -759,6 +745,14 @@
cacheSyncService) =>
_outerRepo = outerRepo;

// No separate GUID cache is needed because PerformGet/PerformGetAll delegate entirely
// to the outer repo's GetMany(), which caches the full dataset via FullDataSetRepositoryCachePolicy.
// GuidReadRepositoryCachePolicy is not suitable here because FullDataSetRepositoryCachePolicy.ClearAll()
// only clears its own entry (exact key removal), so GUID-prefixed entries would become stale
// after Create/Update/Delete operations on the outer repository.
protected override IRepositoryCachePolicy<ITemplate, Guid> CreateCachePolicy()
=> NoCacheRepositoryCachePolicy<ITemplate, Guid>.Instance;

protected override ITemplate? PerformGet(Guid id)
{
// Use the outer repository's GetMany() which benefits from FullDataSetRepositoryCachePolicy.
Expand Down Expand Up @@ -797,42 +791,6 @@
protected override string GetBaseWhereClause() =>
throw new InvalidOperationException("This method won't be implemented.");

/// <summary>
/// Populates the GUID-keyed cache with the given entity.
/// This allows entities retrieved by int ID to also be cached for GUID lookups.
/// </summary>
public void PopulateCacheByKey(ITemplate entity)
{
if (entity.HasIdentity)
{
var cacheKey = GetCacheKey(entity.Key);
IsolatedCache.Insert(cacheKey, () => entity, TimeSpan.FromMinutes(5), true);
}
}

/// <summary>
/// Populates the GUID-keyed cache with the given entities.
/// This allows entities retrieved by int ID to also be cached for GUID lookups.
/// </summary>
public void PopulateCacheByKey(IEnumerable<ITemplate> entities)
{
foreach (ITemplate entity in entities)
{
PopulateCacheByKey(entity);
}
}

/// <summary>
/// Clears the GUID-keyed cache entry for the given key.
/// This ensures deleted entities are not returned from the cache.
/// </summary>
public void ClearCacheByKey(Guid key)
{
var cacheKey = GetCacheKey(key);
IsolatedCache.Clear(cacheKey);
}

private static string GetCacheKey(Guid key) => RepositoryCacheKeys.GetKey<ITemplate>() + key;
}

#endregion
Expand Down
Loading
Loading