umbraco / Umbraco-CMS

Umbraco is a free and open source .NET content management system helping you deliver delightful digital experiences.
https://umbraco.com
MIT License
4.48k stars 2.69k forks source link

.NET 7 OutputCache middleware doesn't work #13384

Closed matthewcare closed 1 year ago

matthewcare commented 1 year ago

Which Umbraco version are you using? (Please write the exact version, example: 10.1.0)

11.0.0-rc1.preview.90

Bug summary

New .NET 7 OutputCachingMiddleware doesn't work with Umbraco.

Specifics

Using the [OutputCache] attribute on controllers, or adding output cache to mapped routes doesn't work with Umbraco. Changing to a minimal hosting approach so that code samples from Microsoft are easier to follow

using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Web.Common.DependencyInjection;

WebApplicationBuilder builder = WebApplication.CreateBuilder(args).ConfigureUmbracoDefaults();

builder.WebHost.UseStaticWebAssets();
builder.Services.AddUmbraco(builder.Environment, builder.Configuration)
    .AddBackOffice()
    .AddWebsite()
    .AddComposers()
    .Build();

builder.Services.AddOutputCache();

WebApplication app = builder.Build();

StaticServiceProvider.Instance = app.Services;
app.Services.GetRequiredService<IRuntimeState>().DetermineRuntimeLevel();

app.UseOutputCache();

app.MapGet("/", () => Task.FromResult(DateTime.Now.ToLongTimeString())).CacheOutput();

app.UseUmbraco()
    .WithMiddleware(u =>
    {
        u.UseBackOffice();
        u.UseWebsite();
    })
    .WithEndpoints(u =>
    {
        u.UseInstallerEndpoints();
        u.UseBackOfficeEndpoints();
        u.UseWebsiteEndpoints();
    });

app.Run();

When viewing the page no caching is present, however when removing app.UseUmbraco() caching works as expected.

Steps to reproduce

As described. Change Program.cs to the provided minimal hosting model (output caching doesn't appear to work at all in the old style).

Observe that the time changes on each page reload.

Comment / remove app.UseUmbraco().... and observe that the time is now cached.

Expected result / actual result

No response

matthewcare commented 1 year ago

After some investigation (with the minimal hosting model at least). It seems that UmbracoApplicationBuilder is the issue. Specifically https://github.com/umbraco/Umbraco-CMS/blob/v11/contrib/src/Umbraco.Web.Common/ApplicationBuilder/UmbracoApplicationBuilder.cs#L108

According to migration documentation this is no longer needed.

When using the minimal hosting model, the endpoint routing middleware wraps the entire middleware pipeline, therefore there's no need to have explicit calls to UseRouting or UseEndpoints to register routes. UseRouting can still be used to specify where route matching happens, but UseRouting doesn't need to be explicitly called if routes should be matched at the beginning of the middleware pipeline.

Speaking with @nul800sebastiaan the minimal hosting model is the preferred approach in Umbraco 11, with #13303 being a hopeful "get it for v11", so I'm not going to investigate the old model.

bergmania commented 1 year ago

Im not sure minimal hosting model work with Umbraco Yet.. IIRC there are issues with when hosted services are started when using minimal hosting model.

matthewcare commented 1 year ago

Is it on the roadmap to be fixed? As time moves on code samples no longer use the old model which makes for a bad developer experience to backport. Also as I found, certain new features also don't work and have no obvious way to resolve the issue.

bergmania commented 1 year ago

We are aware MS have updated code samples.. It is not officially on any roadmaps, but we plan to look into it, to get an idea about the how big a change it require be to support it. If it is simple, we will of cause just fix it, but if is big and requires breaking changes, we will most likely not do anything before v12 or v13.

Zeegaan commented 1 year ago

@matthewcare As the minimal hosting model does not work with Umbraco yet, would you mind swapping the code-samples so I can investigate this 🙈

elit0451 commented 1 year ago

Hi @matthewcare, Thanks for your input and research. We are planning to investigate options with the minimal hosting model, so until we have more information on our end, I am going to close this issue.

bergmania commented 1 year ago

I just tested out Output caching with Umbraco, and seems to work just fine when using route hijacking, which is required to add the OutputCacheAttribute on the page.

My code sample is attached below, and works when having a Document Type named Root. In my example I just printed DateTimeOffset.Now directly from the template.

using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.ViewEngines;
using Microsoft.AspNetCore.OutputCaching;
using Umbraco.Cms.Core.Composing;
using Umbraco.Cms.Core.Web;
using Umbraco.Cms.Web.Common.ApplicationBuilder;
using Umbraco.Cms.Web.Common.Controllers;

namespace Umbraco.Cms.Web.UI;

public class OutputCacheComposer : IComposer
{
    public void Compose(IUmbracoBuilder builder)
    {
        builder.Services.AddOutputCache(options =>
        {
            options.AddPolicy(RootController.CachePolicyName, b =>
            {
                b.Expire(TimeSpan.FromSeconds(10));
                b.Cache();
            });
        });

        builder.Services.Configure<UmbracoPipelineOptions>(options =>
        {
            options.AddFilter(new UmbracoPipelineFilter(
                "OutputCache",
                applicationBuilder =>
                {
                },
                applicationBuilder =>
                {
                    applicationBuilder.UseOutputCache();
                },
                applicationBuilder =>
                {
                    // Add your endpoints here..
                }
            ));
        });
    }
}

public class RootController : RenderController
{

    public const string CachePolicyName = "Test";
    public RootController(ILogger<RenderController> logger, ICompositeViewEngine compositeViewEngine, IUmbracoContextAccessor umbracoContextAccessor) : base(logger, compositeViewEngine, umbracoContextAccessor)
    {
    }

    [OutputCache(PolicyName = CachePolicyName)]
    public override IActionResult Index()
    {
        return base.Index();
    }
}
d-gibbs commented 1 year ago

@bergmania - Thanks for this, it works perfectly. Just as a heads up to others though... we're using a plugin (SEO checker) which isn't compatible with this approach due to the way it registers termininating middleware (endpoints). If you're using this plugin too it wont work for you at the moment. I've reached out to the plugin developer to see if we can get a fix for the problem.