From 297ceaf0aa7ae53ed892fb847f28e2989224601f Mon Sep 17 00:00:00 2001 From: Sven Geusens Date: Wed, 23 Apr 2025 07:09:00 +0200 Subject: [PATCH 1/3] Remove fake null checks as they are no longer needed after merge into v16 (#19109) --- .../BlockListElementLevelVariationTests.Editing.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/PropertyEditors/BlockListElementLevelVariationTests.Editing.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/PropertyEditors/BlockListElementLevelVariationTests.Editing.cs index d7e8ffc5cb..c6f2dff383 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/PropertyEditors/BlockListElementLevelVariationTests.Editing.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/PropertyEditors/BlockListElementLevelVariationTests.Editing.cs @@ -555,8 +555,7 @@ internal partial class BlockListElementLevelVariationTests content = ContentService.GetById(content.Key); var savedBlocksValue = content?.Properties["blocks"]?.GetValue()?.ToString(); - Assert.NotNull(savedBlocksValue); - blockListValue = JsonSerializer.Deserialize(savedBlocksValue); + blockListValue = savedBlocksValue is null ? null : JsonSerializer.Deserialize(savedBlocksValue); // limited user access means invariant data is inaccessible since AllowEditInvariantFromNonDefault is disabled if (updateWithLimitedUserAccess) @@ -973,7 +972,7 @@ internal partial class BlockListElementLevelVariationTests } else { - Assert.AreEqual("null", savedBlocksValue); + Assert.IsNull(savedBlocksValue); } } From cb1ec988ceeba340c5256951c0b0e9e289b0d593 Mon Sep 17 00:00:00 2001 From: Mole Date: Thu, 24 Apr 2025 13:15:30 +0200 Subject: [PATCH 2/3] 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)); + } +} From b4528cf96369f6cc8cd92c49ea9237db37683341 Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Thu, 24 Apr 2025 13:18:29 +0200 Subject: [PATCH 3/3] Fixed error with reflection on integration test configure builder attributes, so integration tests can be created outside of the Umbraco integration test project (#19077) * Fixed error with reflection on integration test configure builder attributes, so integration tests can be created outside of the Umbraco integration test project. * Fix nullability --------- Co-authored-by: mole --- .../Testing/UmbracoIntegrationTest.cs | 41 +++++++++++++++---- 1 file changed, 32 insertions(+), 9 deletions(-) diff --git a/tests/Umbraco.Tests.Integration/Testing/UmbracoIntegrationTest.cs b/tests/Umbraco.Tests.Integration/Testing/UmbracoIntegrationTest.cs index 585f6b268c..ca5654eadd 100644 --- a/tests/Umbraco.Tests.Integration/Testing/UmbracoIntegrationTest.cs +++ b/tests/Umbraco.Tests.Integration/Testing/UmbracoIntegrationTest.cs @@ -189,25 +189,48 @@ public abstract class UmbracoIntegrationTest : UmbracoIntegrationTestBase private void ExecuteBuilderAttributes(IUmbracoBuilder builder) { - // todo better errors + Type? testClassType = GetTestClassType() + ?? throw new Exception($"Could not find test class for {TestContext.CurrentContext.Test.FullName} in order to execute builder attributes."); - // execute builder attributes defined on method - foreach (ConfigureBuilderAttribute builderAttribute in Type.GetType(TestContext.CurrentContext.Test.ClassName) - .GetMethods().First(m => m.Name == TestContext.CurrentContext.Test.MethodName) - .GetCustomAttributes(typeof(ConfigureBuilderAttribute), true)) + // Execute builder attributes defined on method. + foreach (ConfigureBuilderAttribute builderAttribute in GetConfigureBuilderAttributes(testClassType)) { builderAttribute.Execute(builder); } - // execute builder attributes defined on method with param value passtrough from testcase - foreach (ConfigureBuilderTestCaseAttribute builderAttribute in Type.GetType(TestContext.CurrentContext.Test.ClassName) - .GetMethods().First(m => m.Name == TestContext.CurrentContext.Test.MethodName) - .GetCustomAttributes(typeof(ConfigureBuilderTestCaseAttribute), true)) + // Execute builder attributes defined on method with param value pass through from test case. + foreach (ConfigureBuilderTestCaseAttribute builderAttribute in GetConfigureBuilderAttributes(testClassType)) { builderAttribute.Execute(builder); } } + private static Type? GetTestClassType() + { + string testClassName = TestContext.CurrentContext.Test.ClassName; + + // Try resolving the type name directly (which will work for tests in this assembly). + Type testClass = Type.GetType(testClassName); + if (testClass is not null) + { + return testClass; + } + + // Try scanning the loaded assemblies to see if we can find the class by full name. This will be necessary + // for integration test projects using the base classess provided by Umbraco. + var assemblies = AppDomain.CurrentDomain.GetAssemblies(); + return assemblies + .SelectMany(a => a.GetTypes().Where(t => t.FullName == testClassName)) + .FirstOrDefault(); + } + + private static IEnumerable GetConfigureBuilderAttributes(Type testClassType) + where TAttribute : Attribute => + testClassType + .GetMethods().First(m => m.Name == TestContext.CurrentContext.Test.MethodName) + .GetCustomAttributes(typeof(TAttribute), true) + .Cast(); + /// /// Hook for altering UmbracoBuilder setup ///