From cb1ec988ceeba340c5256951c0b0e9e289b0d593 Mon Sep 17 00:00:00 2001 From: Mole Date: Thu, 24 Apr 2025 13:15:30 +0200 Subject: [PATCH] V15: Ensure elements cache is cleared on subscribers in load balanced scenarios (#19128) * Clear elementscache from cache refreshers * Add very simple test ensuring the elements cache is cleared --------- Co-authored-by: Kenn Jacobsen --- .../Implement/ContentCacheRefresher.cs | 49 ++++++++++++++++++ .../Implement/MediaCacheRefresher.cs | 39 ++++++++++++++ .../CacheRefreshingNotificationHandler.cs | 29 +---------- .../Cache/DistributedCacheRefresherTests.cs | 51 +++++++++++++++++++ 4 files changed, 141 insertions(+), 27 deletions(-) create mode 100644 tests/Umbraco.Tests.Integration/Umbraco.Core/Cache/DistributedCacheRefresherTests.cs diff --git a/src/Umbraco.Core/Cache/Refreshers/Implement/ContentCacheRefresher.cs b/src/Umbraco.Core/Cache/Refreshers/Implement/ContentCacheRefresher.cs index b286eccdb7..4741eedcc4 100644 --- a/src/Umbraco.Core/Cache/Refreshers/Implement/ContentCacheRefresher.cs +++ b/src/Umbraco.Core/Cache/Refreshers/Implement/ContentCacheRefresher.cs @@ -1,3 +1,5 @@ +using Microsoft.Extensions.DependencyInjection; +using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Notifications; @@ -21,9 +23,11 @@ public sealed class ContentCacheRefresher : PayloadCacheRefresherBase()) + { + } + + public ContentCacheRefresher( + AppCaches appCaches, + IJsonSerializer serializer, + IIdKeyMap idKeyMap, + IDomainService domainService, + IEventAggregator eventAggregator, + ICacheRefresherNotificationFactory factory, + IDocumentUrlService documentUrlService, + IDomainCacheService domainCacheService, + IDocumentNavigationQueryService documentNavigationQueryService, + IDocumentNavigationManagementService documentNavigationManagementService, + IContentService contentService, + IPublishStatusManagementService publishStatusManagementService, + IDocumentCacheService documentCacheService, + ICacheManager cacheManager) : base(appCaches, serializer, eventAggregator, factory) { _idKeyMap = idKeyMap; @@ -49,6 +86,11 @@ public sealed class ContentCacheRefresher : PayloadCacheRefresherBase(); AppCaches.RuntimeCache.ClearByKey(CacheKeys.ContentRecycleBinCacheKey); + // Ideally, we'd like to not have to clear the entire cache here. However, this was the existing behavior in NuCache. + // The reason for this is that we have no way to know which elements are affected by the changes or what their keys are. + // This is because currently published elements live exclusively in a JSON blob in the umbracoPropertyData table. + // This means that the only way to resolve these keys is to actually parse this data with a specific value converter, and for all cultures, which is not possible. + // If published elements become their own entities with relations, instead of just property data, we can revisit this. + _cacheManager.ElementsCache.Clear(); + var idsRemoved = new HashSet(); IAppPolicyCache isolatedCache = AppCaches.IsolatedCaches.GetOrCreate(); diff --git a/src/Umbraco.Core/Cache/Refreshers/Implement/MediaCacheRefresher.cs b/src/Umbraco.Core/Cache/Refreshers/Implement/MediaCacheRefresher.cs index 284b248aba..7beeda73b6 100644 --- a/src/Umbraco.Core/Cache/Refreshers/Implement/MediaCacheRefresher.cs +++ b/src/Umbraco.Core/Cache/Refreshers/Implement/MediaCacheRefresher.cs @@ -1,3 +1,5 @@ +using Microsoft.Extensions.DependencyInjection; +using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Notifications; @@ -18,7 +20,9 @@ public sealed class MediaCacheRefresher : PayloadCacheRefresherBase()) + { + } + + public MediaCacheRefresher( + AppCaches appCaches, + IJsonSerializer serializer, + IIdKeyMap idKeyMap, + IEventAggregator eventAggregator, + ICacheRefresherNotificationFactory factory, + IMediaNavigationQueryService mediaNavigationQueryService, + IMediaNavigationManagementService mediaNavigationManagementService, + IMediaService mediaService, + IMediaCacheService mediaCacheService, + ICacheManager cacheManager) : base(appCaches, serializer, eventAggregator, factory) { _idKeyMap = idKeyMap; @@ -36,6 +65,9 @@ public sealed class MediaCacheRefresher : PayloadCacheRefresherBase mediaCache = AppCaches.IsolatedCaches.Get(); + // Ideally, we'd like to not have to clear the entire cache here. However, this was the existing behavior in NuCache. + // The reason for this is that we have no way to know which elements are affected by the changes or what their keys are. + // This is because currently published elements live exclusively in a JSON blob in the umbracoPropertyData table. + // This means that the only way to resolve these keys is to actually parse this data with a specific value converter, and for all cultures, which is not possible. + // If published elements become their own entities with relations, instead of just property data, we can revisit this. + _cacheManager.ElementsCache.Clear(); + foreach (JsonPayload payload in payloads) { if (payload.ChangeTypes == TreeChangeTypes.Remove) diff --git a/src/Umbraco.PublishedCache.HybridCache/NotificationHandlers/CacheRefreshingNotificationHandler.cs b/src/Umbraco.PublishedCache.HybridCache/NotificationHandlers/CacheRefreshingNotificationHandler.cs index 6ff5a5fe1e..1283ce90ec 100644 --- a/src/Umbraco.PublishedCache.HybridCache/NotificationHandlers/CacheRefreshingNotificationHandler.cs +++ b/src/Umbraco.PublishedCache.HybridCache/NotificationHandlers/CacheRefreshingNotificationHandler.cs @@ -25,65 +25,40 @@ internal sealed class CacheRefreshingNotificationHandler : { private readonly IDocumentCacheService _documentCacheService; private readonly IMediaCacheService _mediaCacheService; - private readonly IElementsCache _elementsCache; - private readonly IRelationService _relationService; private readonly IPublishedContentTypeCache _publishedContentTypeCache; public CacheRefreshingNotificationHandler( IDocumentCacheService documentCacheService, IMediaCacheService mediaCacheService, - IElementsCache elementsCache, - IRelationService relationService, IPublishedContentTypeCache publishedContentTypeCache) { _documentCacheService = documentCacheService; _mediaCacheService = mediaCacheService; - _elementsCache = elementsCache; - _relationService = relationService; _publishedContentTypeCache = publishedContentTypeCache; } public async Task HandleAsync(ContentRefreshNotification notification, CancellationToken cancellationToken) - { - ClearElementsCache(); - - await _documentCacheService.RefreshContentAsync(notification.Entity); - } + => await _documentCacheService.RefreshContentAsync(notification.Entity); public async Task HandleAsync(ContentDeletedNotification notification, CancellationToken cancellationToken) { foreach (IContent deletedEntity in notification.DeletedEntities) { - ClearElementsCache(); await _documentCacheService.DeleteItemAsync(deletedEntity); } } public async Task HandleAsync(MediaRefreshNotification notification, CancellationToken cancellationToken) - { - ClearElementsCache(); - await _mediaCacheService.RefreshMediaAsync(notification.Entity); - } + => await _mediaCacheService.RefreshMediaAsync(notification.Entity); public async Task HandleAsync(MediaDeletedNotification notification, CancellationToken cancellationToken) { foreach (IMedia deletedEntity in notification.DeletedEntities) { - ClearElementsCache(); await _mediaCacheService.DeleteItemAsync(deletedEntity); } } - private void ClearElementsCache() - { - // Ideally we'd like to not have to clear the entire cache here. However, this was the existing behavior in NuCache. - // The reason for this is that we have no way to know which elements are affected by the changes. or what their keys are. - // This is because currently published elements lives exclusively in a JSON blob in the umbracoPropertyData table. - // This means that the only way to resolve these keys are to actually parse this data with a specific value converter, and for all cultures, which is not feasible. - // If published elements become their own entities with relations, instead of just property data, we can revisit this, - _elementsCache.Clear(); - } - public Task HandleAsync(ContentTypeRefreshedNotification notification, CancellationToken cancellationToken) { const ContentTypeChangeTypes types // only for those that have been refreshed diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Core/Cache/DistributedCacheRefresherTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Core/Cache/DistributedCacheRefresherTests.cs new file mode 100644 index 0000000000..0b9fd4755d --- /dev/null +++ b/tests/Umbraco.Tests.Integration/Umbraco.Core/Cache/DistributedCacheRefresherTests.cs @@ -0,0 +1,51 @@ +using NUnit.Framework; +using Umbraco.Cms.Core.Cache; +using Umbraco.Cms.Core.Services.Changes; +using Umbraco.Cms.Infrastructure.HybridCache; +using Umbraco.Cms.Tests.Common.Testing; +using Umbraco.Cms.Tests.Integration.Testing; + +namespace Umbraco.Cms.Tests.Integration.Umbraco.Core.Cache; + +// We need to make sure that it's the distributed cache refreshers that refresh the elements cache +// see: https://github.com/umbraco/Umbraco-CMS/issues/18467 +[TestFixture] +[UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerFixture)] +internal sealed class DistributedCacheRefresherTests : UmbracoIntegrationTest +{ + private IElementsCache ElementsCache => GetRequiredService(); + + private ContentCacheRefresher ContentCacheRefresher => GetRequiredService(); + + private MediaCacheRefresher MediaCacheRefresher => GetRequiredService(); + + [Test] + public void DistributedContentCacheRefresherClearsElementsCache() + { + var cacheKey = "test"; + PopulateCache("test"); + + ContentCacheRefresher.Refresh([new ContentCacheRefresher.JsonPayload()]); + + Assert.IsNull(ElementsCache.Get(cacheKey)); + } + + [Test] + public void DistributedMediaCacheRefresherClearsElementsCache() + { + var cacheKey = "test"; + PopulateCache("test"); + + MediaCacheRefresher.Refresh([new MediaCacheRefresher.JsonPayload(1, Guid.NewGuid(), TreeChangeTypes.RefreshAll)]); + + Assert.IsNull(ElementsCache.Get(cacheKey)); + } + + private void PopulateCache(string key) + { + ElementsCache.Get(key, () => new object()); + + // Just making sure something is in the cache now. + Assert.IsNotNull(ElementsCache.Get(key)); + } +}