Output Caching: Correctly gate auto-registration of UseOutputCache() middleware#22897
Conversation
…braco managed caching via configuration is enabled, and not consider existing implementation specific registrations.
|
Claude finished @AndyButland's task in 1m 52s —— View job PR ReviewTarget: Introduces
Suggestions
ApprovedThe fix is correct and tightly scoped. The new marker pattern is a direct analogue of Labels applied: |
UseOutputCache() middleware
There was a problem hiding this comment.
Pull request overview
This PR adjusts Umbraco’s automatic UseOutputCache() middleware registration so it only happens when Umbraco itself has enabled output caching (via Website template caching or Delivery API caching), avoiding unintended middleware registration when an application calls services.AddOutputCache(...) for its own purposes.
Changes:
- Introduces an
IUmbracoManagedOutputCacheMarkerservice as the explicit signal that Umbraco-managed output caching is enabled. - Registers the marker when Website and/or Delivery API output caching is configured.
- Updates
UmbracoApplicationBuilderto gateUseOutputCache()on the marker instead ofIOutputCacheStore.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/Umbraco.Web.Website/DependencyInjection/UmbracoBuilderExtensions.cs |
Registers the marker when Website output caching is enabled. |
src/Umbraco.Web.Common/ApplicationBuilder/UmbracoApplicationBuilder.cs |
Switches middleware auto-registration gate from IOutputCacheStore to the new marker. |
src/Umbraco.Core/DependencyInjection/IUmbracoManagedOutputCacheMarker.cs |
Adds the marker interface + implementation used to signal Umbraco-managed output caching. |
src/Umbraco.Cms.Api.Delivery/DependencyInjection/UmbracoBuilderExtensions.cs |
Registers the marker when Delivery API output caching is enabled. |
NillasKA
left a comment
There was a problem hiding this comment.
Looks good and tests good to me!
…` middleware (#22897) Correct the gating of the call to UseOutputCache() to only proceed Umbraco managed caching via configuration is enabled, and not consider existing implementation specific registrations.
Description
In 17.4 we centralised the registration of the ASP.NET Core output cache middleware in
UmbracoApplicationBuilder.RegisterDefaultRequiredMiddleware(), gated on the presence ofIOutputCacheStorein the container. That gate is too broad —IOutputCacheStoreis registered by any call toservices.AddOutputCache(...), including calls made by the consuming application for its own (non-Umbraco) output caching needs.The result: applications that had their own
services.AddOutputCache(...)+app.UseOutputCache()inProgram.cs(working fine pre-17.4) now get a secondUseOutputCache()registration added by Umbraco, and requests fail with:Fix
Replace the
IOutputCacheStoregate with a dedicated marker,IUmbracoManagedOutputCacheMarker, that is registered only when Umbraco itself enables output caching — viaUmbraco:CMS:Website:OutputCache:EnabledorUmbraco:CMS:DeliveryApi:OutputCache:Enabled. Applications callingservices.AddOutputCache(...)for their own purposes are no longer affected by Umbraco's automatic middleware registration.How to test
Reproduces with the UmbracoNewsDashboard sample project on 17.4.0:
services.AddOutputCache(...)+app.UseOutputCache()inProgram.cs, no Umbraco output cache settings enabled →InvalidOperationExceptionon every request before this fix.