OutputCache configuration - policy validation

[witskeeper]

Background and Motivation

The ASP.NET Core output caching middleware is great, but "limited" in terms of policy validation. Let's start with some code that you can write today in .NET 7:

builder.Services.AddOutputCache(options =>
{
    options.AddPolicy("customPolicy", builder =>  builder.Expire(TimeSpan.FromSeconds(20)));
});

There is no way to validate that customPolicy actually exists. This is useful when configuring multiple routes from configuration such as is the case for YARP. See https://github.com/microsoft/reverse-proxy/pull/2328

Proposed API

It would be preferred to something similar to IAuthorizationPolicyProvider implemented via DefaultAuthorizationPolicyProvider and ICorsPolicyProvider implemented via DefaultCorsPolicyProvider

namespace Microsoft.AspNetCore.OutputCaching;

+ public interface IOutputCachePolicyProvider
+ {
+     ValueTask<IOutputCachePolicy?> GetPolicyAsync(string policyName);
+ }
+
+ public class DefaultOutputCachePolicyProvider : IOutputCachePolicyProvider
+ {
+     private readonly OutputCacheOptions _options;
+     
+     public DefaultOutputCachePolicyProvider(IOptions<OutputCacheOptions> options)
+     {
+     
+     }
+     
+     public ValueTask<IOutputCachePolicy?> GetPolicyAsync(string policyName)
+     {
+         options.NamedPolicies[policyName];
+     }
+ }

OutputCacheOptions.NamedPolicies is internal hence this feature cannot be added in another library or the final application.

Usage Examples

See YARP: https://github.com/microsoft/reverse-proxy/blob/02435e0e2eb3e757fc928d9157cfcc7f2859910b/src/ReverseProxy/Configuration/RouteValidators/OutputCachePolicyValidator.cs#L27-L33

Alternative Designs

None

Risks

None

[witskeeper]

like #45684

[ghost]

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

• The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
• The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
• Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

[halter73]

@sebastienros Have you seen this yet? Do you think it's a good idea?

[sebastienros]

My naive response would be to add a GetPolicy(string name) to the options, but the suggested design would offer more flexibility and allow new patterns to provide custom policies.

Note that the API suggestion seems to be a copy-paste of the RateLimiting one since it's referring to DefaultKeyType which is not part of OutputCache. The returned value should be IOutputCachePolicy.

Update: API suggestion has been fixed

[halter73]

API Review Notes:

• We removed references to DefaultKey in the proposal that appears to be copy-pasted from the rate limiter proposal in https://github.com/dotnet/aspnetcore/issues/45684.
• We don't think the implementation needs to be public.
• Given that this doesn't need to expose a new key type publicly like the rate limiter proposal, we think we can accept this.
• The current middleware should use this interface without accessing the options directly to make replacing the implementation useful.

API Approved!

namespace Microsoft.AspNetCore.OutputCaching;

+ public interface IOutputCachePolicyProvider
+ {
+     ValueTask<IOutputCachePolicy?> GetPolicyAsync(string policyName);
+ }

[onurkanbakirci]

I'm working on it

[onurkanbakirci]

Done #57362

@witskeeper