Performance: Implement key-based caching for data type and template repositories

src/Umbraco.Core/Persistence/Repositories/IDataTypeRepository.cs

@@ -3,11 +3,8 @@
 
 namespace Umbraco.Cms.Core.Persistence.Repositories;
 
-public interface IDataTypeRepository : IReadWriteQueryRepository<int, IDataType>
+public interface IDataTypeRepository : IReadWriteQueryRepository<int, IDataType>, IReadRepository<Guid, IDataType>
 {
-
-    IDataType? Get(Guid key);
-
     IEnumerable<MoveEventInfo<IDataType>> Move(IDataType toMove, EntityContainer? container);
 
     /// <summary>

src/Umbraco.Core/Persistence/Repositories/ITemplateRepository.cs

@@ -2,7 +2,7 @@
 
 namespace Umbraco.Cms.Core.Persistence.Repositories;
 
-public interface ITemplateRepository : IReadWriteQueryRepository<int, ITemplate>, IFileRepository
+public interface ITemplateRepository : IReadWriteQueryRepository<int, ITemplate>, IFileRepository, IReadRepository<Guid, ITemplate>
 {
     ITemplate? Get(string? alias);
 

src/Umbraco.Core/Services/DataTypeService.cs

@@ -1,4 +1,4 @@
 using System.ComponentModel.DataAnnotations;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
 using Umbraco.Cms.Core.DependencyInjection;
@@ -311,19 +311,8 @@
         public Task<IEnumerable<IDataType>> GetAllAsync(params Guid[] keys)
         {
             using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true);
-
-            IQuery<IDataType> query = Query<IDataType>();
-            if (keys.Length > 0)
-            {
-                // Need to use a List here because the expression tree cannot convert the array when used in Contains.
-                // See ExpressionTests.Sql_In().
-                List<Guid> keysAsList = [.. keys];
-                query = query.Where(x => keysAsList.Contains(x.Key));
-            }
-
-            IDataType[] dataTypes = _dataTypeRepository.Get(query).ToArray();
-
-            return Task.FromResult<IEnumerable<IDataType>>(dataTypes);
+            IEnumerable<IDataType> dataTypes = _dataTypeRepository.GetMany(keys);
+            return Task.FromResult(dataTypes);
         }
 
         /// <inheritdoc />
@@ -373,7 +362,7 @@
         public Task<IDataType?> GetAsync(Guid id)
         {
             using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true);
-            IDataType? dataType = GetDataTypeFromRepository(id);
+            IDataType? dataType = _dataTypeRepository.Get(id);
 
             return Task.FromResult(dataType);
         }
@@ -675,7 +664,7 @@
             EventMessages eventMessages = EventMessagesFactory.Get();
             using ICoreScope scope = ScopeProvider.CreateCoreScope();
 
-            IDataType? dataType = GetDataTypeFromRepository(id);
+            IDataType? dataType = _dataTypeRepository.Get(id);
             if (dataType == null)
             {
                 return Attempt.FailWithStatus(DataTypeOperationStatus.NotFound, dataType);
@@ -740,7 +729,7 @@
         {
             using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true);
 
-            IDataType? dataType = GetDataTypeFromRepository(key);
+            IDataType? dataType = _dataTypeRepository.Get(key);
             if (dataType == null)
             {
                 // Is an unexpected response, but returning an empty collection aligns with how we handle retrieval of concrete Umbraco
@@ -909,13 +898,6 @@
             return Attempt.SucceedWithStatus(DataTypeOperationStatus.Success, dataType);
         }
 
-        private IDataType? GetDataTypeFromRepository(Guid id)
-        => _idKeyMap.Value.GetIdForKey(id, UmbracoObjectTypes.DataType) switch
-        {
-            { Success: false } => null,
-            { Result: var intId } => _dataTypeRepository.Get(intId),
-        };
-
         private void Audit(AuditType type, int userId, int objectId) =>
             AuditAsync(type, userId, objectId).GetAwaiter().GetResult();
 

src/Umbraco.Core/Services/TemplateService.cs

@@ -186,16 +186,7 @@ public Task<IEnumerable<ITemplate>> GetAllAsync(params string[] aliases)
     public Task<IEnumerable<ITemplate>> GetAllAsync(params Guid[] keys)
     {
         using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true);
-
-        IQuery<ITemplate> query = Query<ITemplate>();
-        if (keys.Any())
-        {
-            var keysList = keys.ToList();
-            query = query.Where(x => keysList.Contains(x.Key));
-        }
-
-        IEnumerable<ITemplate> templates = _templateRepository.Get(query).OrderBy(x => x.Name);
-
+        IEnumerable<ITemplate> templates = _templateRepository.GetMany(keys);
         return Task.FromResult(templates);
     }
 
@@ -229,11 +220,9 @@ public Task<IEnumerable<ITemplate>> GetChildrenAsync(int masterTemplateId)
     /// <inheritdoc />
     public Task<ITemplate?> GetAsync(Guid id)
     {
-        using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true))
-        {
-            IQuery<ITemplate>? query = Query<ITemplate>().Where(x => x.Key == id);
-            return Task.FromResult(_templateRepository.Get(query)?.SingleOrDefault());
-        }
+        using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true);
+        ITemplate? template = _templateRepository.Get(id);
+        return Task.FromResult(template);
     }
 
     /// <inheritdoc />

src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentTypeCommonRepository.cs

@@ -214,7 +214,7 @@ private void MapTemplates(Dictionary<int, IContentTypeComposition> contentTypes)
         List<ContentTypeTemplateDto>? templateDtos = Database?.Fetch<ContentTypeTemplateDto>(sql1);
 
         // var templates = templateRepository.GetMany(templateDtos.Select(x => x.TemplateNodeId).ToArray()).ToDictionary(x => x.Id, x => x);
-        IEnumerable<ITemplate>? allTemplates = _templateRepository.GetMany();
+        IEnumerable<ITemplate>? allTemplates = _templateRepository.GetMany((int[]?)null);
 
         var templates = allTemplates.ToDictionary(x => x.Id, x => x);
         var templateDtoIx = 0;

src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DataTypeRepository.cs

@@ -1,4 +1,4 @@
 using System.Data;
 using System.Globalization;
 using Microsoft.Extensions.Logging;
 using NPoco;
@@ -7,6 +7,7 @@
 using Umbraco.Cms.Core.Events;
 using Umbraco.Cms.Core.Exceptions;
 using Umbraco.Cms.Core.Models;
+using Umbraco.Cms.Core.Persistence;
 using Umbraco.Cms.Core.Persistence.Querying;
 using Umbraco.Cms.Core.Persistence.Repositories;
 using Umbraco.Cms.Core.PropertyEditors;
@@ -30,6 +31,7 @@
     private readonly PropertyEditorCollection _editors;
     private readonly IConfigurationEditorJsonSerializer _serializer;
     private readonly IDataValueEditorFactory _dataValueEditorFactory;
+    private readonly DataTypeByGuidReadRepository _dataTypeByGuidReadRepository;
 
     public DataTypeRepository(
         IScopeAccessor scopeAccessor,
@@ -53,11 +55,38 @@
         _serializer = serializer;
         _dataValueEditorFactory = dataValueEditorFactory;
         _dataTypeLogger = loggerFactory.CreateLogger<IDataType>();
+        _dataTypeByGuidReadRepository = new DataTypeByGuidReadRepository(
+            this,
+            scopeAccessor,
+            cache,
+            loggerFactory.CreateLogger<DataTypeByGuidReadRepository>(),
+            repositoryCacheVersionService,
+            cacheSyncService);
     }
 
     private Guid NodeObjectTypeId => Constants.ObjectTypes.DataType;
 
-    public IDataType? Get(Guid key) => GetMany().FirstOrDefault(x => x.Key == key);
+    public IDataType? Get(Guid key) => _dataTypeByGuidReadRepository.Get(key);
+
+    IEnumerable<IDataType> IReadRepository<Guid, IDataType>.GetMany(params Guid[]? keys) => _dataTypeByGuidReadRepository.GetMany(keys);
+
+    public bool Exists(Guid id) => _dataTypeByGuidReadRepository.Exists(id);
+
+    public override void Save(IDataType entity)
+    {
+        base.Save(entity);
+
+        // Also populate the GUID cache so subsequent lookups by GUID don't hit the database.
+        _dataTypeByGuidReadRepository.PopulateCacheByKey(entity);
+    }
+
+    public override void Delete(IDataType entity)
+    {
+        base.Delete(entity);
+
+        // Also clear the GUID cache so subsequent lookups by GUID don't return stale data.
+        _dataTypeByGuidReadRepository.ClearCacheByKey(entity.Key);
+    }
 
     public IEnumerable<MoveEventInfo<IDataType>> Move(IDataType toMove, EntityContainer? container)
     {
@@ -224,7 +253,18 @@
 
     #region Overrides of RepositoryBase<int,DataTypeDefinition>
 
-    protected override IDataType? PerformGet(int id) => GetMany(id).FirstOrDefault();
+    protected override IDataType? PerformGet(int id)
+    {
+        IDataType? dataType = GetMany(id).FirstOrDefault();
+
+        if (dataType != null)
+        {
+            // Also populate the GUID cache so subsequent lookups by GUID don't hit the database.
+            _dataTypeByGuidReadRepository.PopulateCacheByKey(dataType);
+        }
+
+        return dataType;
+    }
 
     private string? EnsureUniqueNodeName(string? nodeName, int id = 0)
     {
@@ -273,12 +313,17 @@
         }
 
         List<DataTypeDto>? dtos = Database.Fetch<DataTypeDto>(dataTypeSql);
-        return dtos.Select(x => DataTypeFactory.BuildEntity(
+        IDataType[] dataTypes = dtos.Select(x => DataTypeFactory.BuildEntity(
             x,
             _editors,
             _dataTypeLogger,
             _serializer,
             _dataValueEditorFactory)).ToArray();
+
+        // Also populate the GUID cache so subsequent lookups by GUID don't hit the database.
+        _dataTypeByGuidReadRepository.PopulateCacheByKey(dataTypes);
+
+        return dataTypes;
     }
 
     protected override IEnumerable<IDataType> PerformGetByQuery(IQuery<IDataType> query)
@@ -476,4 +521,162 @@
     }
 
     #endregion
+
+    #region Read Repository implementation for Guid keys
+
+    /// <summary>
+    /// Populates the int-keyed cache with the given entity.
+    /// This allows entities retrieved by GUID to also be cached for int ID lookups.
+    /// </summary>
+    private void PopulateCacheById(IDataType entity)
+    {
+        if (entity.HasIdentity)
+        {
+            var cacheKey = GetCacheKey(entity.Id);
+            IsolatedCache.Insert(cacheKey, () => entity, TimeSpan.FromMinutes(5), true);
+        }
+    }
+
+    /// <summary>
+    /// Populates the int-keyed cache with the given entities.
+    /// This allows entities retrieved by GUID to also be cached for int ID lookups.
+    /// </summary>
+    private void PopulateCacheById(IEnumerable<IDataType> entities)
+    {
+        foreach (IDataType entity in entities)
+        {
+            PopulateCacheById(entity);
+        }
+    }
+
+    private static string GetCacheKey(int id) => RepositoryCacheKeys.GetKey<IDataType>() + id;
+
+    // reading repository purely for looking up by GUID
+    private sealed class DataTypeByGuidReadRepository : EntityRepositoryBase<Guid, IDataType>
+    {
+        private readonly DataTypeRepository _outerRepo;
+
+        public DataTypeByGuidReadRepository(
+            DataTypeRepository outerRepo,
+            IScopeAccessor scopeAccessor,
+            AppCaches cache,
+            ILogger<DataTypeByGuidReadRepository> logger,
+            IRepositoryCacheVersionService repositoryCacheVersionService,
+            ICacheSyncService cacheSyncService)
+            : base(
+                scopeAccessor,
+                cache,
+                logger,
+                repositoryCacheVersionService,
+                cacheSyncService) =>
+            _outerRepo = outerRepo;
+
+        protected override IDataType? PerformGet(Guid id)
+        {
+            Sql<ISqlContext> sql = _outerRepo.GetBaseQuery(false)
+                .Where<NodeDto>(x => x.UniqueId == id);
+
+            DataTypeDto? dto = Database.FirstOrDefault<DataTypeDto>(sql);
+
+            if (dto == null)
+            {
+                return null;
+            }
+
+            IDataType dataType = DataTypeFactory.BuildEntity(
+                dto,
+                _outerRepo._editors,
+                _outerRepo._dataTypeLogger,
+                _outerRepo._serializer,
+                _outerRepo._dataValueEditorFactory);
+
+            // Also populate the int-keyed cache so subsequent lookups by int ID don't hit the database
+            _outerRepo.PopulateCacheById(dataType);
+
+            return dataType;
+        }
+
+        protected override IEnumerable<IDataType> PerformGetAll(params Guid[]? ids)
+        {
+            Sql<ISqlContext> sql = _outerRepo.GetBaseQuery(false);
+            if (ids?.Length > 0)
+            {
+                sql.WhereIn<NodeDto>(x => x.UniqueId, ids);
+            }
+            else
+            {
+                sql.Where<NodeDto>(x => x.NodeObjectType == _outerRepo.NodeObjectTypeId);
+            }
+
+            List<DataTypeDto>? dtos = Database.Fetch<DataTypeDto>(sql);
+            IDataType[] dataTypes = dtos.Select(x => DataTypeFactory.BuildEntity(
+                x,
+                _outerRepo._editors,
+                _outerRepo._dataTypeLogger,
+                _outerRepo._serializer,
+                _outerRepo._dataValueEditorFactory)).ToArray();
+
+            // Also populate the int-keyed cache so subsequent lookups by int ID don't hit the database
+            _outerRepo.PopulateCacheById(dataTypes);
+
+            return dataTypes;
+        }
+
+        protected override IEnumerable<IDataType> PerformGetByQuery(IQuery<IDataType> query) =>
+            throw new InvalidOperationException("This method won't be implemented.");
+
+        protected override IEnumerable<string> GetDeleteClauses() =>
+            throw new InvalidOperationException("This method won't be implemented.");
+
+        protected override void PersistNewItem(IDataType entity) =>
+            throw new InvalidOperationException("This method won't be implemented.");
+
+        protected override void PersistUpdatedItem(IDataType entity) =>
+            throw new InvalidOperationException("This method won't be implemented.");
+
+        protected override Sql<ISqlContext> GetBaseQuery(bool isCount) =>
+            throw new InvalidOperationException("This method won't be implemented.");
+
+        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(IDataType 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<IDataType> entities)
+        {
+            foreach (IDataType 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<IDataType>() + key;
+    }
+
+    #endregion
 }

src/Umbraco.Infrastructure/Persistence/Repositories/Implement/EntityRepositoryBase.cs

@@ -182,11 +182,11 @@ public IEnumerable<TEntity> GetMany(params TId[]? ids)
             // don't query by anything that is a default of T (like a zero)
             // TODO: I think we should enabled this in case accidental calls are made to get all with invalid ids
             // .Where(x => Equals(x, default(TId)) == false)
-            .ToArray();
+            .ToArray() ?? [];
 
         // can't query more than 2000 ids at a time... but if someone is really querying 2000+ entities,
         // the additional overhead of fetching them in groups is minimal compared to the lookup time of each group
-        if (ids?.Length <= Constants.Sql.MaxParameterCount)
+        if (ids.Length <= Constants.Sql.MaxParameterCount)
         {
             return CachePolicy.GetAll(ids, PerformGetAll);
         }