V13/bugfix/partial cache (#19314)

* Make sure that each optional section of the cachekey always starts and ends with a -

* Move secondary logic of clearing the membercaches into its own replacable class

* Regsiter the new implementation

* Add a mock to the integration tests as appCaches are disabled

* Added header comments to components.

* Refactored cache key into a method and exposed for testing.
Added unit tests to verify behaviour.

* Verified also that regex matches only the supplied member and asserted on the key itself.

---------

Co-authored-by: Andy Butland <[email protected]>

Author: Migaroez

src/Umbraco.Core/Cache/PartialViewCacheInvalidators/IMemberPartialViewCacheInvalidator.cs

@@ -0,0 +1,16 @@
+namespace Umbraco.Cms.Core.Cache.PartialViewCacheInvalidators;
+
+/// <summary>
+/// Defines behaviours for clearing of cached partials views that are configured to be cached individually by member.
+/// </summary>
+public interface IMemberPartialViewCacheInvalidator
+{
+    /// <summary>
+    /// Clears the partial view cache items for the specified member ids.
+    /// </summary>
+    /// <param name="memberIds">The member Ids to clear the cache for.</param>
+    /// <remarks>
+    /// Called from the <see cref="MemberCacheRefresher"/> when a member is saved or deleted.
+    /// </remarks>
+    void ClearPartialViewCacheItems(IEnumerable<int> memberIds);
+}

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

@@ -1,5 +1,8 @@
 // using Newtonsoft.Json;
 
+using Microsoft.Extensions.DependencyInjection;
+using Umbraco.Cms.Core.Cache.PartialViewCacheInvalidators;
+using Umbraco.Cms.Core.DependencyInjection;
 using Umbraco.Cms.Core.Events;
 using Umbraco.Cms.Core.Models;
 using Umbraco.Cms.Core.Notifications;
@@ -15,10 +18,37 @@ public sealed class MemberCacheRefresher : PayloadCacheRefresherBase<MemberCache
     public static readonly Guid UniqueId = Guid.Parse("E285DF34-ACDC-4226-AE32-C0CB5CF388DA");
 
     private readonly IIdKeyMap _idKeyMap;
+    private readonly IMemberPartialViewCacheInvalidator _memberPartialViewCacheInvalidator;
+
+    [Obsolete("Use the non obsoleted constructor instead. Scheduled for removal in v17")]
+    public MemberCacheRefresher(
+        AppCaches appCaches,
+        IJsonSerializer serializer,
+        IIdKeyMap idKeyMap,
+        IEventAggregator eventAggregator,
+        ICacheRefresherNotificationFactory factory)
+        : this(
+            appCaches,
+            serializer,
+            idKeyMap,
+            eventAggregator,
+            factory,
+            StaticServiceProvider.Instance.GetRequiredService<IMemberPartialViewCacheInvalidator>())
+    {
+    }
 
-    public MemberCacheRefresher(AppCaches appCaches, IJsonSerializer serializer, IIdKeyMap idKeyMap, IEventAggregator eventAggregator, ICacheRefresherNotificationFactory factory)
-        : base(appCaches, serializer, eventAggregator, factory) =>
+    public MemberCacheRefresher(
+        AppCaches appCaches,
+        IJsonSerializer serializer,
+        IIdKeyMap idKeyMap,
+        IEventAggregator eventAggregator,
+        ICacheRefresherNotificationFactory factory,
+        IMemberPartialViewCacheInvalidator memberPartialViewCacheInvalidator)
+        : base(appCaches, serializer, eventAggregator, factory)
+    {
         _idKeyMap = idKeyMap;
+        _memberPartialViewCacheInvalidator = memberPartialViewCacheInvalidator;
+    }
 
     #region Indirect
 
@@ -67,7 +97,9 @@ public override void Remove(int id)
 
     private void ClearCache(params JsonPayload[] payloads)
     {
-        AppCaches.ClearPartialViewCache();
+        // Clear the partial views cache for all partials that are cached by member, for the updates members.
+        _memberPartialViewCacheInvalidator.ClearPartialViewCacheItems(payloads.Select(p => p.Id));
+
         Attempt<IAppPolicyCache?> memberCache = AppCaches.IsolatedCaches.Get<IMember>();
 
         foreach (JsonPayload p in payloads)

src/Umbraco.Web.Website/Cache/PartialViewCacheInvalidators/MemberPartialViewCacheInvalidator.cs

@@ -0,0 +1,35 @@
+using Umbraco.Cms.Core.Cache;
+using Umbraco.Cms.Core.Cache.PartialViewCacheInvalidators;
+using Umbraco.Extensions;
+
+namespace Umbraco.Cms.Web.Website.Cache.PartialViewCacheInvalidators;
+
+/// <summary>
+/// Implementation of <see cref="IMemberPartialViewCacheInvalidator"/> that only remove cached partial views
+/// that were cached for the specified member(s).
+/// </summary>
+public class MemberPartialViewCacheInvalidator : IMemberPartialViewCacheInvalidator
+{
+    private readonly AppCaches _appCaches;
+
+    /// <summary>
+    /// Initializes a new instance of the <see cref="MemberPartialViewCacheInvalidator"/> class.
+    /// </summary>
+    public MemberPartialViewCacheInvalidator(AppCaches appCaches) => _appCaches = appCaches;
+
+    /// <inheritdoc/>
+    /// <remarks>
+    /// Partial view cache keys follow the following format:
+    ///   [] is optional or only added if the information is available
+    ///   {} is a parameter
+    ///   "Umbraco.Web.PartialViewCacheKey{partialViewName}-[{currentThreadCultureName}-][m{memberId}-][c{contextualKey}-]"
+    /// See <see cref="HtmlHelperRenderExtensions.CachedPartialAsync"/> for more information.
+    /// </remarks>
+    public void ClearPartialViewCacheItems(IEnumerable<int> memberIds)
+    {
+        foreach (var memberId in memberIds)
+        {
+            _appCaches.RuntimeCache.ClearByRegex($"{CoreCacheHelperExtensions.PartialViewCacheKey}.*-m{memberId}-*");
+        }
+    }
+}

src/Umbraco.Web.Website/DependencyInjection/UmbracoBuilderExtensions.cs

@@ -5,6 +5,7 @@
 using Microsoft.Extensions.DependencyInjection.Extensions;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
+using Umbraco.Cms.Core.Cache.PartialViewCacheInvalidators;
 using Umbraco.Cms.Core.Configuration.Models;
 using Umbraco.Cms.Core.DependencyInjection;
 using Umbraco.Cms.Core.Routing;
@@ -13,6 +14,7 @@
 using Umbraco.Cms.Infrastructure.DependencyInjection;
 using Umbraco.Cms.Web.Common.Middleware;
 using Umbraco.Cms.Web.Common.Routing;
+using Umbraco.Cms.Web.Website.Cache.PartialViewCacheInvalidators;
 using Umbraco.Cms.Web.Website.Collections;
 using Umbraco.Cms.Web.Website.Models;
 using Umbraco.Cms.Web.Website.Routing;
@@ -74,6 +76,9 @@ public static IUmbracoBuilder AddWebsite(this IUmbracoBuilder builder)
         builder.Services.AddSingleton<IPublicAccessRequestHandler, PublicAccessRequestHandler>();
         builder.Services.AddSingleton<BasicAuthenticationMiddleware>();
 
+        // Partial view cache invalidators
+        builder.Services.AddUnique<IMemberPartialViewCacheInvalidator, MemberPartialViewCacheInvalidator>();
+
         builder
             .AddDistributedCache()
             .AddModelsBuilder();

src/Umbraco.Web.Website/Extensions/HtmlHelperRenderExtensions.cs

@@ -104,7 +104,45 @@ public static IHtmlContent PreviewBadge(
         ViewDataDictionary? viewData = null,
         Func<object, ViewDataDictionary?, string>? contextualKeyBuilder = null)
     {
-        var cacheKey = new StringBuilder(partialViewName);
+        IUmbracoContextAccessor umbracoContextAccessor = GetRequiredService<IUmbracoContextAccessor>(htmlHelper);
+        umbracoContextAccessor.TryGetUmbracoContext(out IUmbracoContext? umbracoContext);
+
+        string cacheKey = await GenerateCacheKeyForCachedPartialViewAsync(
+            partialViewName,
+            cacheByPage,
+            umbracoContext,
+            cacheByMember,
+            cacheByMember ? GetRequiredService<IMemberManager>(htmlHelper) : null,
+            model,
+            viewData,
+            contextualKeyBuilder);
+
+        AppCaches appCaches = GetRequiredService<AppCaches>(htmlHelper);
+        IHostingEnvironment hostingEnvironment = GetRequiredService<IHostingEnvironment>(htmlHelper);
+
+        return appCaches.CachedPartialView(
+            hostingEnvironment,
+            umbracoContext!,
+            htmlHelper,
+            partialViewName,
+            model,
+            cacheTimeout,
+            cacheKey.ToString(),
+            viewData);
+    }
+
+    // Internal for tests.
+    internal static async Task<string> GenerateCacheKeyForCachedPartialViewAsync(
+        string partialViewName,
+        bool cacheByPage,
+        IUmbracoContext? umbracoContext,
+        bool cacheByMember,
+        IMemberManager? memberManager,
+        object model,
+        ViewDataDictionary? viewData,
+        Func<object, ViewDataDictionary?, string>? contextualKeyBuilder)
+    {
+        var cacheKey = new StringBuilder(partialViewName + "-");
 
         // let's always cache by the current culture to allow variants to have different cache results
         var cultureName = Thread.CurrentThread.CurrentUICulture.Name;
@@ -113,24 +151,25 @@ public static IHtmlContent PreviewBadge(
             cacheKey.AppendFormat("{0}-", cultureName);
         }
 
-        IUmbracoContextAccessor umbracoContextAccessor = GetRequiredService<IUmbracoContextAccessor>(htmlHelper);
-        umbracoContextAccessor.TryGetUmbracoContext(out IUmbracoContext? umbracoContext);
-
         if (cacheByPage)
         {
             if (umbracoContext == null)
             {
                 throw new InvalidOperationException(
-                    "Cannot cache by page if the UmbracoContext has not been initialized, this parameter can only be used in the context of an Umbraco request");
+                    "Cannot cache by page if the UmbracoContext has not been initialized, this parameter can only be used in the context of an Umbraco request.");
             }
 
             cacheKey.AppendFormat("{0}-", umbracoContext.PublishedRequest?.PublishedContent?.Id ?? 0);
         }
 
         if (cacheByMember)
         {
-            IMemberManager memberManager =
-                htmlHelper.ViewContext.HttpContext.RequestServices.GetRequiredService<IMemberManager>();
+            if (memberManager == null)
+            {
+                throw new InvalidOperationException(
+                    "Cannot cache by member if the MemberManager is not available.");
+            }
+
             MemberIdentityUser? currentMember = await memberManager.GetCurrentMemberAsync();
             cacheKey.AppendFormat("m{0}-", currentMember?.Id ?? "0");
         }
@@ -141,18 +180,7 @@ public static IHtmlContent PreviewBadge(
             cacheKey.AppendFormat("c{0}-", contextualKey);
         }
 
-        AppCaches appCaches = GetRequiredService<AppCaches>(htmlHelper);
-        IHostingEnvironment hostingEnvironment = GetRequiredService<IHostingEnvironment>(htmlHelper);
-
-        return appCaches.CachedPartialView(
-            hostingEnvironment,
-            umbracoContext!,
-            htmlHelper,
-            partialViewName,
-            model,
-            cacheTimeout,
-            cacheKey.ToString(),
-            viewData);
+        return cacheKey.ToString();
     }
 
     // public static IHtmlContent EditorFor<T>(this IHtmlHelper htmlHelper, string templateName = "", string htmlFieldName = "", object additionalViewData = null)

tests/Umbraco.Tests.Integration/DependencyInjection/UmbracoBuilderExtensions.cs

@@ -11,6 +11,7 @@
 using Moq;
 using NUnit.Framework;
 using Umbraco.Cms.Core.Cache;
+using Umbraco.Cms.Core.Cache.PartialViewCacheInvalidators;
 using Umbraco.Cms.Core.Composing;
 using Umbraco.Cms.Core.Configuration.Models;
 using Umbraco.Cms.Core.DistributedLocking;
@@ -43,6 +44,8 @@ public static class UmbracoBuilderExtensions
     public static IUmbracoBuilder AddTestServices(this IUmbracoBuilder builder, TestHelper testHelper)
     {
         builder.Services.AddUnique(AppCaches.NoCache);
+        builder.Services.AddUnique(Mock.Of<IMemberPartialViewCacheInvalidator>());
+
         builder.Services.AddUnique(Mock.Of<IUmbracoBootPermissionChecker>());
         builder.Services.AddUnique(testHelper.MainDom);
 

tests/Umbraco.Tests.UnitTests/Umbraco.Web.Website/Cache/PartialViewCacheInvalidators/MemberPartialViewCacheInvalidatorTests.cs

@@ -0,0 +1,76 @@
+using System.Text.RegularExpressions;
+using Microsoft.AspNetCore.Mvc.ModelBinding;
+using Microsoft.AspNetCore.Mvc.ViewFeatures;
+using Moq;
+using NUnit.Framework;
+using Umbraco.Cms.Core.Cache;
+using Umbraco.Cms.Core.Security;
+using Umbraco.Cms.Core.Web;
+using Umbraco.Cms.Web.Website.Cache.PartialViewCacheInvalidators;
+using Umbraco.Extensions;
+
+namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Website.Routing;
+
+[TestFixture]
+public class MemberPartialViewCacheInvalidatorTests
+{
+    [Test]
+    public void ClearPartialViewCacheItems_Clears_ExpectedItems()
+    {
+        var runTimeCacheMock = new Mock<IAppPolicyCache>();
+        runTimeCacheMock
+            .Setup(x => x.ClearByRegex(It.IsAny<string>()))
+            .Verifiable();
+        var appCaches = new AppCaches(
+            runTimeCacheMock.Object,
+            NoAppCache.Instance,
+            new IsolatedCaches(type => new ObjectCacheAppCache()));
+        var memberPartialViewCacheInvalidator = new MemberPartialViewCacheInvalidator(appCaches);
+
+        var memberIds = new[] { 1, 2, 3 };
+
+        memberPartialViewCacheInvalidator.ClearPartialViewCacheItems(memberIds);
+
+        foreach (var memberId in memberIds)
+        {
+            var regex = $"Umbraco.Web.PartialViewCacheKey.*-m{memberId}-*";
+            runTimeCacheMock
+                .Verify(x => x.ClearByRegex(It.Is<string>(x => x == regex)), Times.Once);
+        }
+    }
+
+    [Test]
+    public async Task ClearPartialViewCacheItems_Regex_Matches_CachedKeys()
+    {
+        const int MemberId = 1234;
+
+        var memberManagerMock = new Mock<IMemberManager>();
+        memberManagerMock
+            .Setup(x => x.GetCurrentMemberAsync())
+            .ReturnsAsync(new MemberIdentityUser { Id = MemberId.ToString() });
+
+        var cacheKey = await HtmlHelperRenderExtensions.GenerateCacheKeyForCachedPartialViewAsync(
+            "TestPartial.cshtml",
+            true,
+            Mock.Of<IUmbracoContext>(),
+            true,
+            memberManagerMock.Object,
+            new TestViewModel(),
+            new ViewDataDictionary(new EmptyModelMetadataProvider(), new ModelStateDictionary()),
+            null);
+        cacheKey = CoreCacheHelperExtensions.PartialViewCacheKey + cacheKey;
+        Assert.AreEqual("Umbraco.Web.PartialViewCacheKeyTestPartial.cshtml-en-US-0-m1234-", cacheKey);
+
+        var regexForMember = $"Umbraco.Web.PartialViewCacheKey.*-m{MemberId}-*";
+        var regexMatch = Regex.IsMatch(cacheKey, regexForMember);
+        Assert.IsTrue(regexMatch);
+
+        var regexForAnotherMember = $"Umbraco.Web.PartialViewCacheKey.*-m{4321}-*";
+        regexMatch = Regex.IsMatch(cacheKey, regexForAnotherMember);
+        Assert.IsFalse(regexMatch);
+    }
+
+    private class TestViewModel
+    {
+    }
+}