From 467e55c5aa0fdf1eefc961686778592c7542fe55 Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Wed, 20 Aug 2025 09:48:50 +0100 Subject: [PATCH] Refactoring DatabaseCacheRepository to de-duplicate code and remove warnings (#19942) * Refactoring to de-duplicate code and remove warnings. * Update src/Umbraco.PublishedCache.HybridCache/Persistence/DatabaseCacheRepository.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../Persistence/DatabaseCacheRepository.cs | 267 ++++++++---------- .../Persistence/IDatabaseCacheRepository.cs | 3 + 2 files changed, 120 insertions(+), 150 deletions(-) diff --git a/src/Umbraco.PublishedCache.HybridCache/Persistence/DatabaseCacheRepository.cs b/src/Umbraco.PublishedCache.HybridCache/Persistence/DatabaseCacheRepository.cs index 6e1df8b5d0..6923110a2b 100644 --- a/src/Umbraco.PublishedCache.HybridCache/Persistence/DatabaseCacheRepository.cs +++ b/src/Umbraco.PublishedCache.HybridCache/Persistence/DatabaseCacheRepository.cs @@ -21,6 +21,7 @@ using static Umbraco.Cms.Core.Persistence.SqlExtensionsStatics; namespace Umbraco.Cms.Infrastructure.HybridCache.Persistence; +/// internal sealed class DatabaseCacheRepository : RepositoryBase, IDatabaseCacheRepository { private readonly IContentCacheDataSerializerFactory _contentCacheDataSerializerFactory; @@ -58,9 +59,11 @@ internal sealed class DatabaseCacheRepository : RepositoryBase, IDatabaseCacheRe _nucacheSettings = nucacheSettings; } + /// public async Task DeleteContentItemAsync(int id) - => await Database.ExecuteAsync("DELETE FROM cmsContentNu WHERE nodeId=@id", new { id = id }); + => await Database.ExecuteAsync("DELETE FROM cmsContentNu WHERE nodeId = @id", new { id }); + /// public async Task RefreshContentAsync(ContentCacheNode contentCacheNode, PublishedState publishedState) { IContentCacheDataSerializer serializer = _contentCacheDataSerializerFactory.Create(ContentCacheDataSerializerEntityType.Document); @@ -69,7 +72,8 @@ internal sealed class DatabaseCacheRepository : RepositoryBase, IDatabaseCacheRe if (contentCacheNode.IsDraft) { await OnRepositoryRefreshed(serializer, contentCacheNode, true); - // if it's a draft node we don't need to worry about the published state + + // If it's a draft node we don't need to worry about the published state. return; } @@ -79,11 +83,12 @@ internal sealed class DatabaseCacheRepository : RepositoryBase, IDatabaseCacheRe await OnRepositoryRefreshed(serializer, contentCacheNode, false); break; case PublishedState.Unpublishing: - await Database.ExecuteAsync("DELETE FROM cmsContentNu WHERE nodeId=@id AND published=1", new { id = contentCacheNode.Id }); + await Database.ExecuteAsync("DELETE FROM cmsContentNu WHERE nodeId = @id AND published = 1", new { id = contentCacheNode.Id }); break; } } + /// public async Task RefreshMediaAsync(ContentCacheNode contentCacheNode) { IContentCacheDataSerializer serializer = _contentCacheDataSerializerFactory.Create(ContentCacheDataSerializerEntityType.Media); @@ -107,15 +112,7 @@ internal sealed class DatabaseCacheRepository : RepositoryBase, IDatabaseCacheRe mediaTypeIds is not null && mediaTypeIds.Count == 0 && memberTypeIds is not null && memberTypeIds.Count == 0) { - if (Database.DatabaseType == DatabaseType.SqlServer2012) - { - Database.Execute($"TRUNCATE TABLE cmsContentNu"); - } - - if (Database.DatabaseType == DatabaseType.SQLite) - { - Database.Execute($"DELETE FROM cmsContentNu"); - } + TruncateContent(); } RebuildContentDbCache(serializer, _nucacheSettings.Value.SqlPageSize, contentTypeIds); @@ -123,6 +120,20 @@ internal sealed class DatabaseCacheRepository : RepositoryBase, IDatabaseCacheRe RebuildMemberDbCache(serializer, _nucacheSettings.Value.SqlPageSize, memberTypeIds); } + private void TruncateContent() + { + if (Database.DatabaseType == DatabaseType.SqlServer2012) + { + Database.Execute($"TRUNCATE TABLE cmsContentNu"); + } + + if (Database.DatabaseType == DatabaseType.SQLite) + { + Database.Execute($"DELETE FROM cmsContentNu"); + } + } + + /// public async Task GetContentSourceAsync(Guid key, bool preview = false) { Sql? sql = SqlContentSourcesSelect() @@ -147,6 +158,7 @@ internal sealed class DatabaseCacheRepository : RepositoryBase, IDatabaseCacheRe return CreateContentNodeKit(dto, serializer, preview); } + /// public async Task> GetContentSourcesAsync(IEnumerable keys, bool preview = false) { Sql? sql = SqlContentSourcesSelect() @@ -170,7 +182,7 @@ internal sealed class DatabaseCacheRepository : RepositoryBase, IDatabaseCacheRe private IEnumerable GetContentSourceByDocumentTypeKey(IEnumerable documentTypeKeys, Guid objectType) { Guid[] keys = documentTypeKeys.ToArray(); - if (keys.Any() is false) + if (keys.Length == 0) { return []; } @@ -182,14 +194,15 @@ internal sealed class DatabaseCacheRepository : RepositoryBase, IDatabaseCacheRe : throw new ArgumentOutOfRangeException(nameof(objectType), objectType, null); sql.InnerJoin("n") - .On((n, c) => n.NodeId == c.ContentTypeId, "n", "umbracoContent") - .Append(SqlObjectTypeNotTrashed(SqlContext, objectType)) - .WhereIn(x => x.UniqueId, keys,"n") - .Append(SqlOrderByLevelIdSortOrder(SqlContext)); + .On((n, c) => n.NodeId == c.ContentTypeId, "n", "umbracoContent") + .Append(SqlObjectTypeNotTrashed(SqlContext, objectType)) + .WhereIn(x => x.UniqueId, keys,"n") + .Append(SqlOrderByLevelIdSortOrder(SqlContext)); return GetContentNodeDtos(sql); } + /// public IEnumerable GetContentByContentTypeKey(IEnumerable keys, ContentCacheDataSerializerEntityType entityType) { Guid objectType = entityType switch @@ -222,6 +235,7 @@ internal sealed class DatabaseCacheRepository : RepositoryBase, IDatabaseCacheRe public IEnumerable GetDocumentKeysByContentTypeKeys(IEnumerable keys, bool published = false) => GetContentSourceByDocumentTypeKey(keys, Constants.ObjectTypes.Document).Where(x => x.Published == published).Select(x => x.Key); + /// public async Task GetMediaSourceAsync(Guid key) { Sql? sql = SqlMediaSourcesSelect() @@ -241,6 +255,7 @@ internal sealed class DatabaseCacheRepository : RepositoryBase, IDatabaseCacheRe return CreateMediaNodeKit(dto, serializer); } + /// public async Task> GetMediaSourcesAsync(IEnumerable keys) { Sql? sql = SqlMediaSourcesSelect() @@ -262,13 +277,12 @@ internal sealed class DatabaseCacheRepository : RepositoryBase, IDatabaseCacheRe private async Task OnRepositoryRefreshed(IContentCacheDataSerializer serializer, ContentCacheNode content, bool preview) { - // use a custom SQL to update row version on each update - // db.InsertOrUpdate(dto); + ContentNuDto dto = GetDtoFromCacheNode(content, !preview, serializer); await Database.InsertOrUpdateAsync( dto, - "SET data=@data, dataRaw=@dataRaw, rv=rv+1 WHERE nodeId=@id AND published=@published", + "SET data = @data, dataRaw = @dataRaw, rv = rv + 1 WHERE nodeId = @id AND published = @published", new { dataRaw = dto.RawData ?? Array.Empty(), @@ -278,7 +292,12 @@ internal sealed class DatabaseCacheRepository : RepositoryBase, IDatabaseCacheRe }); } - // assumes content tree lock + /// + /// Rebuilds the content database cache for documents. + /// + /// + /// Assumes content tree lock. + /// private void RebuildContentDbCache(IContentCacheDataSerializer serializer, int groupSize, IReadOnlyCollection? contentTypeIds) { if (contentTypeIds is null) @@ -288,55 +307,35 @@ internal sealed class DatabaseCacheRepository : RepositoryBase, IDatabaseCacheRe Guid contentObjectType = Constants.ObjectTypes.Document; - // remove all - if anything fails the transaction will rollback + // Remove all - if anything fails the transaction will rollback. if (contentTypeIds.Count == 0) { - // must support SQL-CE - Database.Execute( - @"DELETE FROM cmsContentNu -WHERE cmsContentNu.nodeId IN ( - SELECT id FROM umbracoNode WHERE umbracoNode.nodeObjectType=@objType -)", - new { objType = contentObjectType }); + DeleteForObjectType(contentObjectType); } else { - // assume number of ctypes won't blow IN(...) - // must support SQL-CE - Database.Execute( - $@"DELETE FROM cmsContentNu -WHERE cmsContentNu.nodeId IN ( - SELECT id FROM umbracoNode - JOIN {Constants.DatabaseSchema.Tables.Content} ON {Constants.DatabaseSchema.Tables.Content}.nodeId=umbracoNode.id - WHERE umbracoNode.nodeObjectType=@objType - AND {Constants.DatabaseSchema.Tables.Content}.contentTypeId IN (@ctypes) -)", - new { objType = contentObjectType, ctypes = contentTypeIds }); + DeleteForObjectTypeAndContentTypes(contentObjectType, contentTypeIds); } - // insert back - if anything fails the transaction will rollback - IQuery query = SqlContext.Query(); - if (contentTypeIds.Count > 0) - { - query = query.WhereIn(x => x.ContentTypeId, contentTypeIds); // assume number of ctypes won't blow IN(...) - } + // Insert back - if anything fails the transaction will rollback. + IQuery query = GetInsertQuery(contentTypeIds); long pageIndex = 0; long processed = 0; long total; do { - // the tree is locked, counting and comparing to total is safe + // The tree is locked, counting and comparing to total is safe. IEnumerable descendants = _documentRepository.GetPage(query, pageIndex++, groupSize, out total, null, Ordering.By("Path")); var items = new List(); var count = 0; foreach (IContent c in descendants) { - // always the edited version + // Always include the edited version. items.Add(GetDtoFromContent(c, false, serializer)); - // and also the published version if it makes any sense + // And also the published version if the document is published. if (c.Published) { items.Add(GetDtoFromContent(c, true, serializer)); @@ -347,10 +346,16 @@ WHERE cmsContentNu.nodeId IN ( Database.BulkInsertRecords(items); processed += count; - } while (processed < total); + } + while (processed < total); } - // assumes media tree lock + /// + /// Rebuilds the content database cache for media. + /// + /// + /// Assumes media tree lock. + /// private void RebuildMediaDbCache(IContentCacheDataSerializer serializer, int groupSize, IReadOnlyCollection? contentTypeIds) { if (contentTypeIds is null) @@ -360,54 +365,40 @@ WHERE cmsContentNu.nodeId IN ( Guid mediaObjectType = Constants.ObjectTypes.Media; - // remove all - if anything fails the transaction will rollback + // Remove all - if anything fails the transaction will rollback. if (contentTypeIds.Count == 0) { - // must support SQL-CE - Database.Execute( - @"DELETE FROM cmsContentNu -WHERE cmsContentNu.nodeId IN ( - SELECT id FROM umbracoNode WHERE umbracoNode.nodeObjectType=@objType -)", - new { objType = mediaObjectType }); + DeleteForObjectType(mediaObjectType); } else { - // assume number of ctypes won't blow IN(...) - // must support SQL-CE - Database.Execute( - $@"DELETE FROM cmsContentNu -WHERE cmsContentNu.nodeId IN ( - SELECT id FROM umbracoNode - JOIN {Constants.DatabaseSchema.Tables.Content} ON {Constants.DatabaseSchema.Tables.Content}.nodeId=umbracoNode.id - WHERE umbracoNode.nodeObjectType=@objType - AND {Constants.DatabaseSchema.Tables.Content}.contentTypeId IN (@ctypes) -)", - new { objType = mediaObjectType, ctypes = contentTypeIds }); + DeleteForObjectTypeAndContentTypes(mediaObjectType, contentTypeIds); } - // insert back - if anything fails the transaction will rollback - IQuery query = SqlContext.Query(); - if (contentTypeIds.Count > 0) - { - query = query.WhereIn(x => x.ContentTypeId, contentTypeIds); // assume number of ctypes won't blow IN(...) - } + // Insert back - if anything fails the transaction will rollback. + IQuery query = GetInsertQuery(contentTypeIds); long pageIndex = 0; long processed = 0; long total; do { - // the tree is locked, counting and comparing to total is safe + // The tree is locked, counting and comparing to total is safe. IEnumerable descendants = _mediaRepository.GetPage(query, pageIndex++, groupSize, out total, null, Ordering.By("Path")); - var items = descendants.Select(m => GetDtoFromContent(m, false, serializer)).ToArray(); + ContentNuDto[] items = descendants.Select(m => GetDtoFromContent(m, false, serializer)).ToArray(); Database.BulkInsertRecords(items); processed += items.Length; - } while (processed < total); + } + while (processed < total); } - // assumes member tree lock + /// + /// Rebuilds the content database cache for members. + /// + /// + /// Assumes member tree lock. + /// private void RebuildMemberDbCache(IContentCacheDataSerializer serializer, int groupSize, IReadOnlyCollection? contentTypeIds) { if (contentTypeIds is null) @@ -417,38 +408,18 @@ WHERE cmsContentNu.nodeId IN ( Guid memberObjectType = Constants.ObjectTypes.Member; - // remove all - if anything fails the transaction will rollback + // Remove all - if anything fails the transaction will rollback. if (contentTypeIds.Count == 0) { - // must support SQL-CE - Database.Execute( - @"DELETE FROM cmsContentNu -WHERE cmsContentNu.nodeId IN ( - SELECT id FROM umbracoNode WHERE umbracoNode.nodeObjectType=@objType -)", - new { objType = memberObjectType }); + DeleteForObjectType(memberObjectType); } else { - // assume number of ctypes won't blow IN(...) - // must support SQL-CE - Database.Execute( - $@"DELETE FROM cmsContentNu -WHERE cmsContentNu.nodeId IN ( - SELECT id FROM umbracoNode - JOIN {Constants.DatabaseSchema.Tables.Content} ON {Constants.DatabaseSchema.Tables.Content}.nodeId=umbracoNode.id - WHERE umbracoNode.nodeObjectType=@objType - AND {Constants.DatabaseSchema.Tables.Content}.contentTypeId IN (@ctypes) -)", - new { objType = memberObjectType, ctypes = contentTypeIds }); + DeleteForObjectTypeAndContentTypes(memberObjectType, contentTypeIds); } - // insert back - if anything fails the transaction will rollback - IQuery query = SqlContext.Query(); - if (contentTypeIds.Count > 0) - { - query = query.WhereIn(x => x.ContentTypeId, contentTypeIds); // assume number of ctypes won't blow IN(...) - } + // Insert back - if anything fails the transaction will rollback. + IQuery query = GetInsertQuery(contentTypeIds); long pageIndex = 0; long processed = 0; @@ -460,12 +431,46 @@ WHERE cmsContentNu.nodeId IN ( ContentNuDto[] items = descendants.Select(m => GetDtoFromContent(m, false, serializer)).ToArray(); Database.BulkInsertRecords(items); processed += items.Length; - } while (processed < total); + } + while (processed < total); + } + + private void DeleteForObjectType(Guid nodeObjectType) => + Database.Execute( + @" + DELETE FROM cmsContentNu + WHERE cmsContentNu.nodeId IN ( + SELECT id FROM umbracoNode WHERE umbracoNode.nodeObjectType = @objType + )", + new { objType = nodeObjectType }); + + private void DeleteForObjectTypeAndContentTypes(Guid nodeObjectType, IReadOnlyCollection contentTypeIds) => + Database.Execute( + $@" + DELETE FROM cmsContentNu + WHERE cmsContentNu.nodeId IN ( + SELECT id FROM umbracoNode + JOIN {Constants.DatabaseSchema.Tables.Content} ON {Constants.DatabaseSchema.Tables.Content}.nodeId=umbracoNode.id + WHERE umbracoNode.nodeObjectType = @objType + AND {Constants.DatabaseSchema.Tables.Content}.contentTypeId IN (@ctypes) + )", + new { objType = nodeObjectType, ctypes = contentTypeIds }); + + private IQuery GetInsertQuery(IReadOnlyCollection contentTypeIds) + where TContent : IContentBase + { + IQuery query = SqlContext.Query(); + if (contentTypeIds.Count > 0) + { + query = query.WhereIn(x => x.ContentTypeId, contentTypeIds); + } + + return query; } private ContentNuDto GetDtoFromCacheNode(ContentCacheNode cacheNode, bool published, IContentCacheDataSerializer serializer) { - // the dictionary that will be serialized + // Prepare the data structure that will be serialized. var contentCacheData = new ContentCacheDataModel { PropertyData = cacheNode.Data?.Properties, @@ -487,23 +492,19 @@ WHERE cmsContentNu.nodeId IN ( private ContentNuDto GetDtoFromContent(IContentBase content, bool published, IContentCacheDataSerializer serializer) { - // should inject these in ctor - // BUT for the time being we decide not to support ConvertDbToXml/String - // var propertyEditorResolver = PropertyEditorResolver.Current; - // var dataTypeService = ApplicationContext.Current.Services.DataTypeService; var propertyData = new Dictionary(); foreach (IProperty prop in content.Properties) { var pdatas = new List(); foreach (IPropertyValue pvalue in prop.Values.OrderBy(x => x.Culture)) { - // sanitize - properties should be ok but ... never knows + // Sanitize - properties should be ok but ... never knows. if (!prop.PropertyType.SupportsVariation(pvalue.Culture, pvalue.Segment)) { continue; } - // note: at service level, invariant is 'null', but here invariant becomes 'string.Empty' + // Note: at service level, invariant is 'null', but here invariant becomes 'string.Empty' var value = published ? pvalue.PublishedValue : pvalue.EditedValue; if (value != null) { @@ -521,7 +522,7 @@ WHERE cmsContentNu.nodeId IN ( var cultureData = new Dictionary(); - // sanitize - names should be ok but ... never knows + // Sanitize - names should be ok but ... never knows. if (content.ContentType.VariesByCulture()) { ContentCultureInfosCollection? infos = content is IContent document @@ -530,7 +531,6 @@ WHERE cmsContentNu.nodeId IN ( : document.CultureInfos : content.CultureInfos; - // ReSharper disable once UseDeconstruction if (infos is not null) { foreach (ContentCultureInfos cultureInfo in infos) @@ -547,7 +547,7 @@ WHERE cmsContentNu.nodeId IN ( } } - // the dictionary that will be serialized + // Prepare the data structure that will be serialized. var contentCacheData = new ContentCacheDataModel { PropertyData = propertyData, @@ -566,7 +566,6 @@ WHERE cmsContentNu.nodeId IN ( return dto; } - // we want arrays, we want them all loaded, not an enumerable private Sql SqlContentSourcesSelect(Func>? joins = null) { SqlTemplate sqlTemplate = SqlContext.Templates.Get( @@ -632,36 +631,6 @@ WHERE cmsContentNu.nodeId IN ( return sql; } - private Sql SqlContentSourcesSelectUmbracoNodeJoin(ISqlContext sqlContext) - { - ISqlSyntaxProvider syntax = sqlContext.SqlSyntax; - - SqlTemplate sqlTemplate = sqlContext.Templates.Get( - Constants.SqlTemplates.NuCacheDatabaseDataSource.SourcesSelectUmbracoNodeJoin, builder => - builder.InnerJoin("x") - .On( - (left, right) => left.NodeId == right.NodeId || - SqlText(left.Path, right.Path, - (lp, rp) => $"({lp} LIKE {syntax.GetConcat(rp, "',%'")})"), - aliasRight: "x")); - - Sql sql = sqlTemplate.Sql(); - return sql; - } - - private Sql SqlWhereNodeId(ISqlContext sqlContext, int id) - { - ISqlSyntaxProvider syntax = sqlContext.SqlSyntax; - - SqlTemplate sqlTemplate = sqlContext.Templates.Get( - Constants.SqlTemplates.NuCacheDatabaseDataSource.WhereNodeId, - builder => - builder.Where(x => x.NodeId == SqlTemplate.Arg("id"))); - - Sql sql = sqlTemplate.Sql(id); - return sql; - } - private Sql SqlWhereNodeKey(ISqlContext sqlContext, Guid key) { ISqlSyntaxProvider syntax = sqlContext.SqlSyntax; @@ -702,10 +671,8 @@ WHERE cmsContentNu.nodeId IN ( } /// - /// Returns a slightly more optimized query to use for the document counting when paging over the content sources + /// Returns a slightly more optimized query to use for the document counting when paging over the content sources. /// - /// - /// private Sql SqlContentSourcesCount(Func>? joins = null) { SqlTemplate sqlTemplate = SqlContext.Templates.Get( diff --git a/src/Umbraco.PublishedCache.HybridCache/Persistence/IDatabaseCacheRepository.cs b/src/Umbraco.PublishedCache.HybridCache/Persistence/IDatabaseCacheRepository.cs index a10a616fdc..ede7c426d1 100644 --- a/src/Umbraco.PublishedCache.HybridCache/Persistence/IDatabaseCacheRepository.cs +++ b/src/Umbraco.PublishedCache.HybridCache/Persistence/IDatabaseCacheRepository.cs @@ -3,6 +3,9 @@ using Umbraco.Cms.Infrastructure.HybridCache.Serialization; namespace Umbraco.Cms.Infrastructure.HybridCache.Persistence; +/// +/// Defines a repository for persisting content cache data. +/// internal interface IDatabaseCacheRepository { ///