Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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);
}
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
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;
using Umbraco.Cms.Core.Persistence.Repositories;
using Umbraco.Cms.Core.Serialization;
using Umbraco.Cms.Core.Services;
using Umbraco.Extensions;

namespace Umbraco.Cms.Core.Cache;

Expand All @@ -13,10 +15,32 @@
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 contructor instead. Planned for removal in V18")]
public MemberCacheRefresher(AppCaches appCaches, IJsonSerializer serializer, IIdKeyMap idKeyMap, IEventAggregator eventAggregator, ICacheRefresherNotificationFactory factory)
: base(appCaches, serializer, eventAggregator, factory) =>
: this(
appCaches,
serializer,
idKeyMap,
eventAggregator,
factory,
StaticServiceProvider.Instance.GetRequiredService<IMemberPartialViewCacheInvalidator>())
{
}

public MemberCacheRefresher(
AppCaches appCaches,
IJsonSerializer serializer,
IIdKeyMap idKeyMap,
IEventAggregator eventAggregator,
ICacheRefresherNotificationFactory factory,
IMemberPartialViewCacheInvalidator memberPartialViewCacheInvalidator)
: base(appCaches, serializer, eventAggregator, factory)
{
_idKeyMap = idKeyMap;
_memberPartialViewCacheInvalidator = memberPartialViewCacheInvalidator;
}

Check warning on line 43 in src/Umbraco.Core/Cache/Refreshers/Implement/MemberCacheRefresher.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

❌ New issue: Constructor Over-Injection

MemberCacheRefresher has 6 arguments, threshold = 5. This constructor has too many arguments, indicating an object with low cohesion or missing function argument abstraction. Avoid adding more arguments.

#region Indirect

Expand Down Expand Up @@ -65,7 +89,8 @@

private void ClearCache(params JsonPayload[] payloads)
{
AppCaches.ClearPartialViewCache();
_memberPartialViewCacheInvalidator.ClearPartialViewCacheItems(payloads.Select(p => p.Id));

Attempt<IAppPolicyCache?> memberCache = AppCaches.IsolatedCaches.Get<IMember>();

foreach (JsonPayload p in payloads)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
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}-*");
}

// since it is possible to add a cache item linked to members without a member logged in, we should always clear these items.
_appCaches.RuntimeCache.ClearByRegex($"{CoreCacheHelperExtensions.PartialViewCacheKey}.*-m-*");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -73,6 +75,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();
Expand Down
66 changes: 47 additions & 19 deletions src/Umbraco.Web.Website/Extensions/HtmlHelperRenderExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Net;

Check notice on line 1 in src/Umbraco.Web.Website/Extensions/HtmlHelperRenderExtensions.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

✅ Getting better: Primitive Obsession

The ratio of primitive types in function arguments decreases from 55.94% to 55.78%, threshold = 30.0%. The functions in this file have too many primitive types (e.g. int, double, float) in their function argument lists. Using many primitive types lead to the code smell Primitive Obsession. Avoid adding more primitive arguments.
using System.Text;
using System.Text.Encodings.Web;
using System.Web;
Expand Down Expand Up @@ -105,55 +105,83 @@
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;
if (!string.IsNullOrEmpty(cultureName))
{
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");
}

if (contextualKeyBuilder != null)
{
var contextualKey = contextualKeyBuilder(model, viewData);
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();

Check warning on line 184 in src/Umbraco.Web.Website/Extensions/HtmlHelperRenderExtensions.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

❌ New issue: Complex Method

GenerateCacheKeyForCachedPartialViewAsync has a cyclomatic complexity of 9, threshold = 9. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.

Check warning on line 184 in src/Umbraco.Web.Website/Extensions/HtmlHelperRenderExtensions.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

❌ New issue: Bumpy Road Ahead

GenerateCacheKeyForCachedPartialViewAsync has 2 blocks with nested conditional logic. Any nesting of 2 or deeper is considered. Threshold is one single, nested block per function. The Bumpy Road code smell is a function that contains multiple chunks of nested conditional logic. The deeper the nesting and the more bumps, the lower the code health.
}

// public static IHtmlContent EditorFor<T>(this IHtmlHelper htmlHelper, string templateName = "", string htmlFieldName = "", object additionalViewData = null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using NUnit.Framework;
using Umbraco.Cms.Core;
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;
Expand Down Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
@@ -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
{
}
}