yv01p/Umbraco-CMS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

V15: Only cache items if all ancestors are published (#18337)

Browse Source 
* Introduce IsDocumentPublishedInAnyCulture

Sometimes we don't care about culture

* Check ancestor path when resolving cache items

* Fix tests

* Rebuild NavigationService

* Only set node if it has a published ancestor path

* Remove branch when unpublished

* Add tests

* Add seed test

* Consider published ancestor path when seeding documents

* Introduce MediaBreadthFirstKeyProviderTests

This is needed since the logic of document and media is no longer the same

* Remove unused services

* Move assert page to helper

* Add variant tests

* Add tests

* Filter keys in ContentTypeSeedKeyProvider

* Fix tests

* Add failing test showing refreshing issue

* Don't blow up if we can't resolve the node from navigation cache

Turns out that this can actually happen :D Should be fine to just return false

* Refactor cache refresher check

* Make NavigationQueryService service protected

* Add comment on how to refactor breadth first key provider

* Refactor if statement
This commit is contained in:
Mole
2025-02-17 12:51:33 +01:00
committed by GitHub
parent 69e251ad17
commit c76d764598
15 changed files with 680 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
src/Umbraco.PublishedCache.HybridCache/SeedKeyProviders/BreadthFirstKeyProvider.cs
View File

@@ -4,12 +4,12 @@ namespace Umbraco.Cms.Infrastructure.HybridCache.SeedKeyProviders;
public abstract class BreadthFirstKeyProvider
{
private readonly INavigationQueryService _navigationQueryService;
protected readonly INavigationQueryService NavigationQueryService;
private readonly int _seedCount;
public BreadthFirstKeyProvider(INavigationQueryService navigationQueryService, int seedCount)
{
_navigationQueryService = navigationQueryService;
NavigationQueryService = navigationQueryService;
_seedCount = seedCount;
}
@@ -24,7 +24,7 @@ public abstract class BreadthFirstKeyProvider
HashSet<Guid> keys = [];
int keyCount = 0;
if (_navigationQueryService.TryGetRootKeys(out IEnumerable<Guid> rootKeys) is false)
if (NavigationQueryService.TryGetRootKeys(out IEnumerable<Guid> rootKeys) is false)
{
return new HashSet<Guid>();
}
@@ -44,7 +44,7 @@ public abstract class BreadthFirstKeyProvider
{
Guid key = keyQueue.Dequeue();
if (_navigationQueryService.TryGetChildrenKeys(key, out IEnumerable<Guid> childKeys) is false)
if (NavigationQueryService.TryGetChildrenKeys(key, out IEnumerable<Guid> childKeys) is false)
{
continue;
}

11
src/Umbraco.PublishedCache.HybridCache/SeedKeyProviders/Document/ContentTypeSeedKeyProvider.cs
View File

@@ -1,6 +1,7 @@
using Microsoft.Extensions.Options;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Scoping;
using Umbraco.Cms.Core.Services.Navigation;
using Umbraco.Cms.Infrastructure.HybridCache.Persistence;
namespace Umbraco.Cms.Infrastructure.HybridCache.SeedKeyProviders.Document;
@@ -9,22 +10,28 @@ internal sealed class ContentTypeSeedKeyProvider : IDocumentSeedKeyProvider
{
private readonly ICoreScopeProvider _scopeProvider;
private readonly IDatabaseCacheRepository _databaseCacheRepository;
private readonly IPublishStatusQueryService _publishStatusService;
private readonly CacheSettings _cacheSettings;
public ContentTypeSeedKeyProvider(
ICoreScopeProvider scopeProvider,
IDatabaseCacheRepository databaseCacheRepository,
IOptions<CacheSettings> cacheSettings)
IOptions<CacheSettings> cacheSettings,
IPublishStatusQueryService publishStatusService)
{
_scopeProvider = scopeProvider;
_databaseCacheRepository = databaseCacheRepository;
_publishStatusService = publishStatusService;
_cacheSettings = cacheSettings.Value;
}
public ISet<Guid> GetSeedKeys()
{
using ICoreScope scope = _scopeProvider.CreateCoreScope();
var documentKeys = _databaseCacheRepository.GetDocumentKeysByContentTypeKeys(_cacheSettings.ContentTypeKeys, published: true).ToHashSet();
var documentKeys = _databaseCacheRepository
.GetDocumentKeysByContentTypeKeys(_cacheSettings.ContentTypeKeys, published: true)
.Where(key => _publishStatusService.IsDocumentPublishedInAnyCulture(key))
.ToHashSet();
scope.Complete();
return documentKeys;

70
src/Umbraco.PublishedCache.HybridCache/SeedKeyProviders/Document/DocumentBreadthFirstKeyProvider.cs
View File

@@ -6,10 +6,78 @@ namespace Umbraco.Cms.Infrastructure.HybridCache.SeedKeyProviders.Document;
internal sealed class DocumentBreadthFirstKeyProvider : BreadthFirstKeyProvider, IDocumentSeedKeyProvider
{
private readonly IPublishStatusQueryService _publishStatusService;
private readonly int _seedCount;
public DocumentBreadthFirstKeyProvider(
IDocumentNavigationQueryService documentNavigationQueryService,
IOptions<CacheSettings> cacheSettings)
IOptions<CacheSettings> cacheSettings,
IPublishStatusQueryService publishStatusService)
: base(documentNavigationQueryService, cacheSettings.Value.DocumentBreadthFirstSeedCount)
{
_publishStatusService = publishStatusService;
_seedCount = cacheSettings.Value.DocumentBreadthFirstSeedCount;
}
// TODO: V16 - Move this method back to the base class
// The main need for this is because we now need to filter the keys, based on if they have published ancestor path or not
// We should add `FilterKeys` virtual method on the base class that does nothing, and then override it here instead
// Note that it's important that we do this filtering as we're doing the search, since we want to make sure we hit the seed count
// For instance if you have 500 content nodes, request 100 seeded, we need to return 100 keys, even if we need to filter out 20 of them
public new ISet<Guid> GetSeedKeys()
{
if (_seedCount == 0)
{
return new HashSet<Guid>();
}
Queue<Guid> keyQueue = new();
HashSet<Guid> keys = [];
int keyCount = 0;
if (NavigationQueryService.TryGetRootKeys(out IEnumerable<Guid> rootKeys) is false)
{
return new HashSet<Guid>();
}
rootKeys = rootKeys.Where(x => _publishStatusService.IsDocumentPublishedInAnyCulture(x));
foreach (Guid key in rootKeys)
{
keyCount++;
keys.Add(key);
keyQueue.Enqueue(key);
if (keyCount == _seedCount)
{
return keys;
}
}
while (keyQueue.Count > 0 && keyCount < _seedCount)
{
Guid key = keyQueue.Dequeue();
if (NavigationQueryService.TryGetChildrenKeys(key, out IEnumerable<Guid> childKeys) is false)
{
continue;
}
childKeys = childKeys.Where(x => _publishStatusService.IsDocumentPublishedInAnyCulture(x));
foreach (Guid childKey in childKeys)
{
keys.Add(childKey);
keyCount++;
if (keyCount == _seedCount)
{
return keys;
}
keyQueue.Enqueue(childKey);
}
}
return keys;
}
}

61
src/Umbraco.PublishedCache.HybridCache/Services/DocumentCacheService.cs
View File

@@ -6,6 +6,7 @@ using Umbraco.Cms.Core.Models.PublishedContent;
using Umbraco.Cms.Core.PublishedCache;
using Umbraco.Cms.Core.Scoping;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Core.Services.Navigation;
using Umbraco.Cms.Infrastructure.HybridCache.Factories;
using Umbraco.Cms.Infrastructure.HybridCache.Persistence;
using Umbraco.Cms.Infrastructure.HybridCache.Serialization;
@@ -24,8 +25,11 @@ internal sealed class DocumentCacheService : IDocumentCacheService
private readonly IEnumerable<IDocumentSeedKeyProvider> _seedKeyProviders;
private readonly IPublishedModelFactory _publishedModelFactory;
private readonly IPreviewService _previewService;
private readonly IPublishStatusQueryService _publishStatusQueryService;
private readonly IDocumentNavigationQueryService _documentNavigationQueryService;
private readonly CacheSettings _cacheSettings;
private HashSet<Guid>? _seedKeys;
private HashSet<Guid> SeedKeys
{
get
@@ -56,7 +60,9 @@ internal sealed class DocumentCacheService : IDocumentCacheService
IEnumerable<IDocumentSeedKeyProvider> seedKeyProviders,
IOptions<CacheSettings> cacheSettings,
IPublishedModelFactory publishedModelFactory,
IPreviewService previewService)
IPreviewService previewService,
IPublishStatusQueryService publishStatusQueryService,
IDocumentNavigationQueryService documentNavigationQueryService)
{
_databaseCacheRepository = databaseCacheRepository;
_idKeyMap = idKeyMap;
@@ -67,6 +73,8 @@ internal sealed class DocumentCacheService : IDocumentCacheService
_seedKeyProviders = seedKeyProviders;
_publishedModelFactory = publishedModelFactory;
_previewService = previewService;
_publishStatusQueryService = publishStatusQueryService;
_documentNavigationQueryService = documentNavigationQueryService;
_cacheSettings = cacheSettings.Value;
}
@@ -101,6 +109,20 @@ internal sealed class DocumentCacheService : IDocumentCacheService
{
using ICoreScope scope = _scopeProvider.CreateCoreScope();
ContentCacheNode? contentCacheNode = await _databaseCacheRepository.GetContentSourceAsync(key, preview);
// If we can resolve the content cache node, we still need to check if the ancestor path is published.
// This does cost some performance, but it's necessary to ensure that the content is actually published.
// When unpublishing a node, a payload with RefreshBranch is published, so we don't have to worry about this.
// Similarly, when a branch is published, next time the content is requested, the parent will be published,
// this works because we don't cache null values.
if (preview is false && contentCacheNode is not null)
{
if (HasPublishedAncestorPath(contentCacheNode.Key) is false)
{
return null;
}
}
scope.Complete();
return contentCacheNode;
},
@@ -116,6 +138,28 @@ internal sealed class DocumentCacheService : IDocumentCacheService
return _publishedContentFactory.ToIPublishedContent(contentCacheNode, preview).CreateModel(_publishedModelFactory);
}
private bool HasPublishedAncestorPath(Guid contentKey)
{
var success = _documentNavigationQueryService.TryGetAncestorsKeys(contentKey, out IEnumerable<Guid> keys);
if (success is false)
{
// This might happen is certain cases, since 0notifications are not ordered, for instance, if you save and publish a content node in the same scope.
// In this case we'll try and update the node in the cache even though it hasn't been updated in the document navigation cache yet.
// It's okay to just return false here, since the node will be loaded later when it's actually requested.
return false;
}
foreach (Guid key in keys)
{
if (_publishStatusQueryService.IsDocumentPublishedInAnyCulture(key) is false)
{
return false;
}
}
return true;
}
private bool GetPreview()
{
return _previewService.IsInPreview();
@@ -169,7 +213,7 @@ internal sealed class DocumentCacheService : IDocumentCacheService
}
ContentCacheNode? publishedNode = await _databaseCacheRepository.GetContentSourceAsync(key, false);
if (publishedNode is not null)
if (publishedNode is not null && HasPublishedAncestorPath(publishedNode.Key))
{
await _hybridCache.SetAsync(GetCacheKey(publishedNode.Key, false), publishedNode, GetEntryOptions(publishedNode.Key));
}
@@ -195,7 +239,7 @@ internal sealed class DocumentCacheService : IDocumentCacheService
var cacheKey = GetCacheKey(key, false);
// We'll use GetOrCreateAsync because it may be in the second level cache, in which case we don't have to re-seed.
ContentCacheNode? cachedValue = await _hybridCache.GetOrCreateAsync<ContentCacheNode?>(
ContentCacheNode? cachedValue = await _hybridCache.GetOrCreateAsync<ContentCacheNode?>(
cacheKey,
async cancel =>
{
@@ -212,17 +256,20 @@ internal sealed class DocumentCacheService : IDocumentCacheService
return cacheNode;
},
GetSeedEntryOptions(),
cancellationToken: cancellationToken);
GetSeedEntryOptions(),
cancellationToken: cancellationToken);
// If the value is null, it's likely because
if (cachedValue is null)
if (cachedValue is null)
{
await _hybridCache.RemoveAsync(cacheKey);
await _hybridCache.RemoveAsync(cacheKey, cancellationToken);
}
}
}
// Internal for test purposes.
internal void ResetSeedKeys() => _seedKeys = null;
private HybridCacheEntryOptions GetSeedEntryOptions() => new()
{
Expiration = _cacheSettings.Entry.Document.SeedCacheDuration,