Skip to content

Commit cb1ec98

Browse files
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 <[email protected]>
1 parent 297ceaf commit cb1ec98

File tree

4 files changed

+141
-27
lines changed

4 files changed

+141
-27
lines changed

src/Umbraco.Core/Cache/Refreshers/Implement/ContentCacheRefresher.cs

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
using Microsoft.Extensions.DependencyInjection;
2+
using Umbraco.Cms.Core.DependencyInjection;
13
using Umbraco.Cms.Core.Events;
24
using Umbraco.Cms.Core.Models;
35
using Umbraco.Cms.Core.Notifications;
@@ -21,9 +23,11 @@ public sealed class ContentCacheRefresher : PayloadCacheRefresherBase<ContentCac
2123
private readonly IDocumentNavigationManagementService _documentNavigationManagementService;
2224
private readonly IContentService _contentService;
2325
private readonly IDocumentCacheService _documentCacheService;
26+
private readonly ICacheManager _cacheManager;
2427
private readonly IPublishStatusManagementService _publishStatusManagementService;
2528
private readonly IIdKeyMap _idKeyMap;
2629

30+
[Obsolete("Use the constructor with ICacheManager instead, scheduled for removal in V17.")]
2731
public ContentCacheRefresher(
2832
AppCaches appCaches,
2933
IJsonSerializer serializer,
@@ -38,6 +42,39 @@ public ContentCacheRefresher(
3842
IContentService contentService,
3943
IPublishStatusManagementService publishStatusManagementService,
4044
IDocumentCacheService documentCacheService)
45+
: this(
46+
appCaches,
47+
serializer,
48+
idKeyMap,
49+
domainService,
50+
eventAggregator,
51+
factory,
52+
documentUrlService,
53+
domainCacheService,
54+
documentNavigationQueryService,
55+
documentNavigationManagementService,
56+
contentService,
57+
publishStatusManagementService,
58+
documentCacheService,
59+
StaticServiceProvider.Instance.GetRequiredService<ICacheManager>())
60+
{
61+
}
62+
63+
public ContentCacheRefresher(
64+
AppCaches appCaches,
65+
IJsonSerializer serializer,
66+
IIdKeyMap idKeyMap,
67+
IDomainService domainService,
68+
IEventAggregator eventAggregator,
69+
ICacheRefresherNotificationFactory factory,
70+
IDocumentUrlService documentUrlService,
71+
IDomainCacheService domainCacheService,
72+
IDocumentNavigationQueryService documentNavigationQueryService,
73+
IDocumentNavigationManagementService documentNavigationManagementService,
74+
IContentService contentService,
75+
IPublishStatusManagementService publishStatusManagementService,
76+
IDocumentCacheService documentCacheService,
77+
ICacheManager cacheManager)
4178
: base(appCaches, serializer, eventAggregator, factory)
4279
{
4380
_idKeyMap = idKeyMap;
@@ -49,6 +86,11 @@ public ContentCacheRefresher(
4986
_contentService = contentService;
5087
_documentCacheService = documentCacheService;
5188
_publishStatusManagementService = publishStatusManagementService;
89+
90+
// TODO: Ideally we should inject IElementsCache
91+
// this interface is in infrastructure, and changing this is very breaking
92+
// so as long as we have the cache manager, which casts the IElementsCache to a simple AppCache we might as well use that.
93+
_cacheManager = cacheManager;
5294
}
5395

5496
#region Indirect
@@ -83,6 +125,13 @@ public override void Refresh(JsonPayload[] payloads)
83125
AppCaches.RuntimeCache.ClearOfType<PublicAccessEntry>();
84126
AppCaches.RuntimeCache.ClearByKey(CacheKeys.ContentRecycleBinCacheKey);
85127

128+
// Ideally, we'd like to not have to clear the entire cache here. However, this was the existing behavior in NuCache.
129+
// The reason for this is that we have no way to know which elements are affected by the changes or what their keys are.
130+
// This is because currently published elements live exclusively in a JSON blob in the umbracoPropertyData table.
131+
// 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.
132+
// If published elements become their own entities with relations, instead of just property data, we can revisit this.
133+
_cacheManager.ElementsCache.Clear();
134+
86135
var idsRemoved = new HashSet<int>();
87136
IAppPolicyCache isolatedCache = AppCaches.IsolatedCaches.GetOrCreate<IContent>();
88137

src/Umbraco.Core/Cache/Refreshers/Implement/MediaCacheRefresher.cs

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
using Microsoft.Extensions.DependencyInjection;
2+
using Umbraco.Cms.Core.DependencyInjection;
13
using Umbraco.Cms.Core.Events;
24
using Umbraco.Cms.Core.Models;
35
using Umbraco.Cms.Core.Notifications;
@@ -18,7 +20,9 @@ public sealed class MediaCacheRefresher : PayloadCacheRefresherBase<MediaCacheRe
1820
private readonly IMediaNavigationManagementService _mediaNavigationManagementService;
1921
private readonly IMediaService _mediaService;
2022
private readonly IMediaCacheService _mediaCacheService;
23+
private readonly ICacheManager _cacheManager;
2124

25+
[Obsolete("Use the constructor with ICacheManager instead, scheduled for removal in V17.")]
2226
public MediaCacheRefresher(
2327
AppCaches appCaches,
2428
IJsonSerializer serializer,
@@ -29,13 +33,41 @@ public MediaCacheRefresher(
2933
IMediaNavigationManagementService mediaNavigationManagementService,
3034
IMediaService mediaService,
3135
IMediaCacheService mediaCacheService)
36+
: this(
37+
appCaches,
38+
serializer,
39+
idKeyMap,
40+
eventAggregator,
41+
factory,
42+
mediaNavigationQueryService,
43+
mediaNavigationManagementService,
44+
mediaService,
45+
mediaCacheService,
46+
StaticServiceProvider.Instance.GetRequiredService<ICacheManager>())
47+
{
48+
}
49+
50+
public MediaCacheRefresher(
51+
AppCaches appCaches,
52+
IJsonSerializer serializer,
53+
IIdKeyMap idKeyMap,
54+
IEventAggregator eventAggregator,
55+
ICacheRefresherNotificationFactory factory,
56+
IMediaNavigationQueryService mediaNavigationQueryService,
57+
IMediaNavigationManagementService mediaNavigationManagementService,
58+
IMediaService mediaService,
59+
IMediaCacheService mediaCacheService,
60+
ICacheManager cacheManager)
3261
: base(appCaches, serializer, eventAggregator, factory)
3362
{
3463
_idKeyMap = idKeyMap;
3564
_mediaNavigationQueryService = mediaNavigationQueryService;
3665
_mediaNavigationManagementService = mediaNavigationManagementService;
3766
_mediaService = mediaService;
3867
_mediaCacheService = mediaCacheService;
68+
69+
// TODO: Use IElementsCache instead of ICacheManager, see ContentCacheRefresher for more information.
70+
_cacheManager = cacheManager;
3971
}
4072

4173
#region Indirect
@@ -87,6 +119,13 @@ public override void Refresh(JsonPayload[]? payloads)
87119
AppCaches.RuntimeCache.ClearByKey(CacheKeys.MediaRecycleBinCacheKey);
88120
Attempt<IAppPolicyCache?> mediaCache = AppCaches.IsolatedCaches.Get<IMedia>();
89121

122+
// Ideally, we'd like to not have to clear the entire cache here. However, this was the existing behavior in NuCache.
123+
// The reason for this is that we have no way to know which elements are affected by the changes or what their keys are.
124+
// This is because currently published elements live exclusively in a JSON blob in the umbracoPropertyData table.
125+
// 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.
126+
// If published elements become their own entities with relations, instead of just property data, we can revisit this.
127+
_cacheManager.ElementsCache.Clear();
128+
90129
foreach (JsonPayload payload in payloads)
91130
{
92131
if (payload.ChangeTypes == TreeChangeTypes.Remove)

src/Umbraco.PublishedCache.HybridCache/NotificationHandlers/CacheRefreshingNotificationHandler.cs

Lines changed: 2 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -25,65 +25,40 @@ internal sealed class CacheRefreshingNotificationHandler :
2525
{
2626
private readonly IDocumentCacheService _documentCacheService;
2727
private readonly IMediaCacheService _mediaCacheService;
28-
private readonly IElementsCache _elementsCache;
29-
private readonly IRelationService _relationService;
3028
private readonly IPublishedContentTypeCache _publishedContentTypeCache;
3129

3230
public CacheRefreshingNotificationHandler(
3331
IDocumentCacheService documentCacheService,
3432
IMediaCacheService mediaCacheService,
35-
IElementsCache elementsCache,
36-
IRelationService relationService,
3733
IPublishedContentTypeCache publishedContentTypeCache)
3834
{
3935
_documentCacheService = documentCacheService;
4036
_mediaCacheService = mediaCacheService;
41-
_elementsCache = elementsCache;
42-
_relationService = relationService;
4337
_publishedContentTypeCache = publishedContentTypeCache;
4438
}
4539

4640
public async Task HandleAsync(ContentRefreshNotification notification, CancellationToken cancellationToken)
47-
{
48-
ClearElementsCache();
49-
50-
await _documentCacheService.RefreshContentAsync(notification.Entity);
51-
}
41+
=> await _documentCacheService.RefreshContentAsync(notification.Entity);
5242

5343
public async Task HandleAsync(ContentDeletedNotification notification, CancellationToken cancellationToken)
5444
{
5545
foreach (IContent deletedEntity in notification.DeletedEntities)
5646
{
57-
ClearElementsCache();
5847
await _documentCacheService.DeleteItemAsync(deletedEntity);
5948
}
6049
}
6150

6251
public async Task HandleAsync(MediaRefreshNotification notification, CancellationToken cancellationToken)
63-
{
64-
ClearElementsCache();
65-
await _mediaCacheService.RefreshMediaAsync(notification.Entity);
66-
}
52+
=> await _mediaCacheService.RefreshMediaAsync(notification.Entity);
6753

6854
public async Task HandleAsync(MediaDeletedNotification notification, CancellationToken cancellationToken)
6955
{
7056
foreach (IMedia deletedEntity in notification.DeletedEntities)
7157
{
72-
ClearElementsCache();
7358
await _mediaCacheService.DeleteItemAsync(deletedEntity);
7459
}
7560
}
7661

77-
private void ClearElementsCache()
78-
{
79-
// Ideally we'd like to not have to clear the entire cache here. However, this was the existing behavior in NuCache.
80-
// The reason for this is that we have no way to know which elements are affected by the changes. or what their keys are.
81-
// This is because currently published elements lives exclusively in a JSON blob in the umbracoPropertyData table.
82-
// 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.
83-
// If published elements become their own entities with relations, instead of just property data, we can revisit this,
84-
_elementsCache.Clear();
85-
}
86-
8762
public Task HandleAsync(ContentTypeRefreshedNotification notification, CancellationToken cancellationToken)
8863
{
8964
const ContentTypeChangeTypes types // only for those that have been refreshed
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
using NUnit.Framework;
2+
using Umbraco.Cms.Core.Cache;
3+
using Umbraco.Cms.Core.Services.Changes;
4+
using Umbraco.Cms.Infrastructure.HybridCache;
5+
using Umbraco.Cms.Tests.Common.Testing;
6+
using Umbraco.Cms.Tests.Integration.Testing;
7+
8+
namespace Umbraco.Cms.Tests.Integration.Umbraco.Core.Cache;
9+
10+
// We need to make sure that it's the distributed cache refreshers that refresh the elements cache
11+
// see: https://github.com/umbraco/Umbraco-CMS/issues/18467
12+
[TestFixture]
13+
[UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerFixture)]
14+
internal sealed class DistributedCacheRefresherTests : UmbracoIntegrationTest
15+
{
16+
private IElementsCache ElementsCache => GetRequiredService<IElementsCache>();
17+
18+
private ContentCacheRefresher ContentCacheRefresher => GetRequiredService<ContentCacheRefresher>();
19+
20+
private MediaCacheRefresher MediaCacheRefresher => GetRequiredService<MediaCacheRefresher>();
21+
22+
[Test]
23+
public void DistributedContentCacheRefresherClearsElementsCache()
24+
{
25+
var cacheKey = "test";
26+
PopulateCache("test");
27+
28+
ContentCacheRefresher.Refresh([new ContentCacheRefresher.JsonPayload()]);
29+
30+
Assert.IsNull(ElementsCache.Get(cacheKey));
31+
}
32+
33+
[Test]
34+
public void DistributedMediaCacheRefresherClearsElementsCache()
35+
{
36+
var cacheKey = "test";
37+
PopulateCache("test");
38+
39+
MediaCacheRefresher.Refresh([new MediaCacheRefresher.JsonPayload(1, Guid.NewGuid(), TreeChangeTypes.RefreshAll)]);
40+
41+
Assert.IsNull(ElementsCache.Get(cacheKey));
42+
}
43+
44+
private void PopulateCache(string key)
45+
{
46+
ElementsCache.Get(key, () => new object());
47+
48+
// Just making sure something is in the cache now.
49+
Assert.IsNotNull(ElementsCache.Get(key));
50+
}
51+
}

0 commit comments

Comments
 (0)