aspnetcore
aspnetcore copied to clipboard
Add tag trough OutputCacheAttribute
NOTE: Commandeering @kellberg's original issue to turn it into an API proposal since folks are already engaged here.
Background and Motivation
Output Caching can be controlled via various APIs including via policy setup in DI, extension methods on the endpoint builder, and attributes on MVC actions and endpoint handler delegates. Cache entries can also be "tagged" which makes it possible to evict sets of cache entries. Unfortunately, the tagging mechanism is not exposed when configuring output caching via attributes.
The proposal is that we fix this short coming with the [OutputCache] attritribute by allowing the following usage:
app.MapGet("/api/points-of-interest", [OutputCache(Tags = new [] { "geospatial" })]() => {});
Proposed API
namespace Microsoft.AspNetCore.OutputCaching;
public sealed class OutputCacheAttribute : Attribute
{
+ public string[]? Tags { get; init; }
}
Usage Examples
Minimal APIs
app.MapGet("/api/points-of-interest", [OutputCache(Tags = new [] { "geospatial" })]() => { ... });
MVC
[OutputCache(Tags = new [] { "geospatial" })]
public PointOfInterest[] GetPointsOfInterest() { }
Alternative Designs
We already have multiple ways of setting output cache values, this just adds a missing one to the [OutputCache(...)] attribute. However I did consider whether we should use the singular Tag as the property, however other properties on the attribute are already pluralized.
Risks
None that I can think of.
Original issue content from OP.
Is there an existing issue for this?
- [X] I have searched the existing issues
Is your feature request related to a problem? Please describe the problem.
I would like to add an output cache tag for cache eviction but the OutputCacheAttribute doesn't have that option.
Describe the solution you'd like
Add a Tag parameter to OutputCacheAttribute and update BuildPolicy to add the tag to the IOutputCachePolicy.
Then you can add the tag through the attribute: [OutputCache(Tags = "test")]
This sounds like a good idea for the sake of consistency.
@captainsafia do you know if small changes like this need to use the formal API review template?
Note to @kellberg in my implementation I used the plural Tags and it tags an array like many of the other properties on the attribute.
@captainsafia do you know if small changes like this need to use the formal API review template?
@halter73 can provide the conclusive decision but I believe that adding a parameter to an existing API does require API review.
@halter73 can provide the conclusive decision but I believe that adding a parameter to an existing API does require API review.
It sure does. We do formal API reviews for anything that touches PublicAPI.Unshipped.txt unless it's just fixing a nullability annotation to more accurately match existing behavior like in #45751. There are even things that don't touch PublicAPI.Unshipped.txt like analyzers and explicitly implemented interfaces that we cover in API review.
@mitchdenny Can you copy the template from https://github.com/dotnet/aspnetcore/issues/new?assignees=&labels=api-suggestion&template=30_api_proposal.md&title= into a new comment on this issue, fill it out, and add the api-ready-for-review label?
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 missed your comment, I updated the initial issue content in this instance ;)
API Review Notes:
- This follows a very similar pattern to other
string[]?properties on theOutputCacheAttribute. - @sebastienros omitted this originally because tag data is often dynamic, but even in our samples, we sometimes use static sets of tags for a given endpoint.
API approved as proposed!
Closing this issue now since the code has been added to main, it will appear in .NET 8.0.