Skip to content

V13/bugfix/partial cache #19314

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
May 14, 2025
Merged

V13/bugfix/partial cache #19314

merged 7 commits into from
May 14, 2025

Conversation

Migaroez
Copy link
Contributor

Prerequisites

  • I have added steps to test this contribution in the description below

Fixes #19098

Description

When caching the execution of a partial view by using the CachedPartialAsync extension method on the IHtmlHelper and stating you want it cached by member or not, it seems logical to expect that when a member is updated, only the partials that are cached for that specific member are removed from the cache.

Since an IMember gets updated every time it logs in, as the last login date is saved, the partial views cache was fully invalidated every time a user logs in resulting in a very ineffective caching strategy under heavy member load.

In future versions we will look into separating the last login date and similar Identity fields from the domain object to lower the amount of domain object persistence operations we need to do but since this would be to breaking for v13 and because being able to extend/replace only the code that deals with the partial view cache invalidation triggered from a member update is a friendly thing to supply we went with this approach for v13.

We have also made plans to look into similar enhancements for the other CacheRefreshers to split the logic to make it easier to extend in the future.

Notes

I placed the implementation into the same project as the code that builds the cache key. As the core should not hold an implementation that is tightly bound to the workings of a web application. In an ideal scenario the core would define a more abstract interface to run extra cache invalidators but that will be looked at when we investigate the rework of the CacheRefreshers.

Testing

  • Setup a member login setup with multiple members
  • Setup up at least 2 partial views that you render on an page, one of them cached by member and the other not. You can find some inspiration below
  • Login with one member, the page should display both partials
  • Login with another member in incognito, the page should display the same result for the non member cached partial and a different result for the member cached partial
  • Reload the page for the first member, their result should not change
  • Logout and login one of the members, the member specific partial should change, the other not
  • Try to break the default implementation by supplying extra parameters to the CachedPartialAsync method or changing the stup
  • Try to replace the default implementation trough composition

Testing code examples

General.cshtml

@inherits Umbraco.Cms.Web.Common.Views.UmbracoViewPage

@{
    <p>Time of first render: @DateTime.Now.ToString("R")</p>
}

Member.cshtml

@inherits Umbraco.Cms.Web.Common.Views.UmbracoViewPage
@{
    <p>Time of first render after login: @DateTime.Now.ToString("R")</p>
}

Master.cshtml

...
@await Html.CachedPartialAsync("general",Model,TimeSpan.FromDays(1))
@if (MemberManager.IsLoggedIn())
{
    @await Html.CachedPartialAsync("member",Model,TimeSpan.FromDays(1), cacheByMember:true)
}
...

Copy link
Contributor

@AndyButland AndyButland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All looks good to me, tests out as expected and will solve the raised issue.

I just added some header comments to help for anyone looking to understand what these new components are for, and added some unit tests. To support these I refactored out the method of generating the cache key into a method we could expose for testing - that then allowed checking the generated regex correctly matches the cache key.

To verify composition of a custom implementation I re-added the previous behaviour with:

using Umbraco.Cms.Core.Cache;
using Umbraco.Cms.Core.Cache.PartialViewCacheInvalidators;

public class MyMemberPartialViewCacheInvalidator : IMemberPartialViewCacheInvalidator
{
    private readonly AppCaches _appCaches;

    public MyMemberPartialViewCacheInvalidator(AppCaches appCaches) => _appCaches = appCaches;

    public void ClearPartialViewCacheItems(IEnumerable<int> memberIds) => _appCaches.ClearPartialViewCache();
}

And:

using Umbraco.Cms.Core.Cache.PartialViewCacheInvalidators;
using Umbraco.Cms.Core.Composing;

public class TestComposer : IComposer
{
    public void Compose(IUmbracoBuilder builder)
    {
        builder.Services.AddUnique<IMemberPartialViewCacheInvalidator, MyMemberPartialViewCacheInvalidator>();
    }
}

@AndyButland AndyButland merged commit 4f1604f into v13/dev May 14, 2025
18 of 19 checks passed
@AndyButland AndyButland deleted the v13/bugfix/partial-cache branch May 14, 2025 06:46
Migaroez added a commit that referenced this pull request Jun 2, 2025
# Resolved merge conflic in src/Umbraco.Core/Cache/Refreshers/Implement/MemberCacheRefresher.cs
Migaroez added a commit that referenced this pull request Jun 2, 2025
# Resolved merge conflic in src/Umbraco.Core/Cache/Refreshers/Implement/MemberCacheRefresher.cs
AndyButland pushed a commit that referenced this pull request Jun 2, 2025
* v16 cherry pick of member partial cache invalidator see #19314

# Resolved merge conflic in src/Umbraco.Core/Cache/Refreshers/Implement/MemberCacheRefresher.cs

* Take nullmember cacheitems into account
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants