dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.46k stars 10.03k forks source link

Response Caching should have a way of clearing a specific resource from the cache #43272

Open ronniebarker opened 2 years ago

ronniebarker commented 2 years ago

Is there an existing issue for this?

Is your feature request related to a problem? Please describe the problem.

I am trying to clear a resource from the response cache because I have a new updated version.

Describe the solution you'd like

To be able to clear a resource from the cache (or, in the worst case, clear the whole thing)

Additional context

No response

sebastienros commented 2 years ago

Output Caching is a new feature in .NET 7 starting from preview 6 that handles this scenario. Can it be something that would work for you. It's driven by configuration and not by headers.

There is an intermediate package on NuGet that works with dotnet 6 if you can try it: https://www.nuget.org/packages/Preview.OutputCaching

ronniebarker commented 2 years ago

Perhaps - but I don't think it is a big enough issue for us yet to arrant trying a preview package. I am managing to clear items by returning a response with a max-age=1 (although the spec says you can use zero, the framework seems to reject that) but that requires a request from the resource which is fine for a single clear, but not for any bulk or service-driven clear.

sebastienros commented 2 years ago

Are you setting the max-age header on the client? And in this case the server action will be hit even if it's cached (for longer in this case) and it will update the cache on its way back?

ronniebarker commented 2 years ago

I am making a request with a custom header, using that header to decide to set max-age on the response so that the response cache is replaced with the "very soon to expire" version - effectively clearing that item in a second.

On Fri, Aug 19, 2022 at 21:52:41, Sébastien Ros @.***> wrote:

Are you setting the max-age header on the client? And in this case the server action will be hit even if it's cached (for longer in this case) and it will update the cache on its way back?

— Reply to this email directly, view it on GitHub https://github.com/dotnet/aspnetcore/issues/43272#issuecomment-1221084116, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAA7RBNSRBPMJJJSLJL3BPDVZ7XZTANCNFSM56O6RWPQ . You are receiving this because you authored the thread.Message ID: @.***>

halter73 commented 2 years ago

There's debate among us in triage on whether you are setting the max-age request header or setting some other header. Are you also setting any response headers? If so, are the response headers set on already-cached responses? Do the headers on already-cached responses affect the cache lifetime on the server?

If you could just provide a sample app that does the cache clearing with the max-age=1 workaround you are using today, that would really help explain the scenario.

ronniebarker commented 2 years ago

So….

I have an image that is generated from some parameters and stored in a database whenever a request is made and it isn't found in the database.

If the internal parameters change so that the image needs to be regenerated then the cache needs to be flushed for that item

I have a management web app sends an api request to clear the image (and actually is probably the same request that changes the parameters) from the database;

then after that response completes it sends a javascript fetch (simulating the browser making an image request) with Cache-Control:no-cache to bust through the cache and hit the main image request handler and also x-generate=no which tells my handler not to generate the actual image, but to return an empty 200 response with Cache-Control: public,max-age=1

The response caching framework caches the empty response and then expires a second later.

The first new request after that second will cause the new image to be generated, stored in the database and then returned and cached by the response caching middleware.

On Mon, Aug 22, 2022 at 23:30:25, Stephen Halter @.***> wrote:

There's debate among us in triage on whether you are setting the max-age request header or setting some other header? Are you also setting any response headers? If so, are the response headers set on already-cached responses? Do the headers on already-cached responses affect the cache lifetime on the server?

If you could just provide a sample app that does the cache clearing with the max-age=1 workaround you are using today, that would really help explain the scenario.

— Reply to this email directly, view it on GitHub https://github.com/dotnet/aspnetcore/issues/43272#issuecomment-1223248302, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAA7RBMY6L2Z7TLCBTOFVXLV2P5QDANCNFSM56O6RWPQ . You are receiving this because you authored the thread.Message ID: @.***>

ronniebarker commented 2 years ago

Sorry - I forgot to add what I would prefer:

  1. For the api call to request the system to change the parameters to be able to clear the cache
  2. For me to be able to regenerate multiple images and clear their caches without having to make multiple requests for each image from a client.

On Mon, Aug 22, 2022 at 23:53:21, Ronnie Barker < @.***> wrote:

So….

I have an image that is generated from some parameters and stored in a database whenever a request is made and it isn't found in the database.

If the internal parameters change so that the image needs to be regenerated then the cache needs to be flushed for that item

I have a management web app sends an api request to clear the image (and actually is probably the same request that changes the parameters) from the database;

then after that response completes it sends a javascript fetch (simulating the browser making an image request) with Cache-Control:no-cache to bust through the cache and hit the main image request handler and also x-generate=no which tells my handler not to generate the actual image, but to return an empty 200 response with Cache-Control: public,max-age=1

The response caching framework caches the empty response and then expires a second later.

The first new request after that second will cause the new image to be generated, stored in the database and then returned and cached by the response caching middleware.

On Mon, Aug 22, 2022 at 23:30:25, Stephen Halter @.***

wrote:

There's debate among us in triage on whether you are setting the max-age request header or setting some other header? Are you also setting any response headers? If so, are the response headers set on already-cached responses? Do the headers on already-cached responses affect the cache lifetime on the server?

If you could just provide a sample app that does the cache clearing with the max-age=1 workaround you are using today, that would really help explain the scenario.

— Reply to this email directly, view it on GitHub https://github.com/dotnet/aspnetcore/issues/43272#issuecomment-1223248302, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAA7RBMY6L2Z7TLCBTOFVXLV2P5QDANCNFSM56O6RWPQ . You are receiving this because you authored the thread.Message ID: @.***>

halter73 commented 2 years ago

Got it. To summarize:

  1. You send a specially crafted Cache-Control: no-cache request to an already-cached endpoint. This bypasses the cache.
  2. If you see this specially crafted request, in response you send Cache-Control: public,max-age=1. This overrides the previous cache entry with one that expires sooner (in 1 second).

Based on this, I have two main observations.

I am managing to clear items by returning a response with a max-age=1 (although the spec says you can use zero, the framework seems to reject that)

This seems like a fair criticism. Given that you can override previous cache entries, why can't max-age=0 indicate both not to cache the response and to evict existing entries? It seems overly prescriptive not to allow this if the spec allows it when there's a valid use case presented here.

Output Caching is a new feature in .NET 7 starting from preview 6 that handles this scenario. Can it be something that would work for you. It's driven by configuration and not by headers.

@sebastienros How does output caching address this scenario? This issue is about having "a way of clearing a specific resource from the cache." I know you could OutputCacheContext.Tags with IOutputCacheStore.EvictByTagAsync, but uniquely tagging each entry would be terribly inefficient.

Is it possible to set OutputCacheContext.ResponseExpirationTimeSpan to TimeSpan.Zero on an already cached response to evict it? That sounds more in line with what @ronniebarker wants.

ronniebarker commented 2 years ago

Yes. That’s right. Looking quickly at the Output Cache preview, clearing tags looks like it will do what I want. Also I think it will deal with my other issue which is the ability to ignore the host when generating the key

On Tue, Aug 23 2022 at 00:55, Stephen Halter @.***> wrote:

Got it. To summarize:

  1. You send a specially crafted Cache-Control: no-cache request to an already-cached endpoint. This bypasses the cache.
  2. If you see this specially crafted request, in response you send Cache-Control: public,max-age=1. This overrides the previous cache entry with one that expires sooner (in 1 second).

Based on this, I have two main observations.

I am managing to clear items by returning a response with a max-age=1 (although the spec says you can use zero, the framework seems to reject that)

This seems like a fair criticism. Given that you can override previous cache entries, why can't max-age=0 indicate both not to cache the response and to evict existing entries? It seems overly prescriptive not to allow this if the spec allows it when there's a valid use case presented here.

Output Caching is a new feature in .NET 7 starting from preview 6 that handles this scenario. Can it be something that would work for you. It's driven by configuration and not by headers.

@sebastienros https://github.com/sebastienros How does output caching address this scenario? This issue is about having "a way of clearing a specific resource from the cache." I know you could OutputCacheContext.Tags with IOutputCacheStore.EvictByTagAsync, but uniquely tagging each entry would be terribly inefficient.

Is it possible to set OutputCacheContext.ResponseExpirationTimeSpan to TimeSpan.Zero on an already cached response to evict it? That sounds more in line with what @ronniebarker https://github.com/ronniebarker wants.

— Reply to this email directly, view it on GitHub https://github.com/dotnet/aspnetcore/issues/43272#issuecomment-1223342340, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAA7RBNNZYXVA6WTLCCSJE3V2QHONANCNFSM56O6RWPQ . You are receiving this because you were mentioned.Message ID: @.***>

halter73 commented 2 years ago

@ronniebarker Would you give each cache entry its own unique tag, or is evicting a multiple entries at once using a tag preferrable? While tags probably could be used to evict single entries, I doubt it would be as efficient as finding the cache entry associated with the current request and adjusting the ResponseExpirationTimeSpan to zero.

ronniebarker commented 2 years ago

I think that by tag would be useful (I have a "code" that could map to a tag that covers a logical group of images). But also I was wondering about some kind of URL/Path/Query/Key matcher which could have wildcards to wipe out batches based on the values used to generate the cache key…

On Tue, Aug 23, 2022 at 01:38:03, Stephen Halter @.***> wrote:

@ronniebarker https://github.com/ronniebarker Would you give each cache entry its own unique tag, or is evicting a multiple entries at once using a tag preferrable? While tags probably could be used to evict single entries, I doubt it would be as efficient as finding the cache entry associated with the current request and adjusting the ResponseExpirationTimeSpan to zero.

— Reply to this email directly, view it on GitHub https://github.com/dotnet/aspnetcore/issues/43272#issuecomment-1223373073, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAA7RBNKDWDEVUQUJDS7YLDV2QMOXANCNFSM56O6RWPQ . You are receiving this because you were mentioned.Message ID: @.***>

adityamandaleeka commented 2 years ago

@ronniebarker We'd love it if you could try out OutputCaching in .NET 7 (RC 1 should ship in September) and let us know how this workflow works for you, or if you have any other feedback about it.

adityamandaleeka commented 2 years ago

Leaving open to collect feedback once you've tried it :smile:

nquandt commented 4 months ago

@adityamandaleeka this might not be the best place to ask this, i can always make a new issue or discussion.. Is it bad practice or are there any performance implications to having a tag per resource?

My use case is I have OutputCaching enabled in YARP and need ways of clearing single pages from my application. My thought was in my custom CachingPolicy I just call context.Tags.Add(context.HttpContext.Request.Path);. That way later I can evict based on paths. Do you see any issues with that?

My full policy

services.AddOutputCache(options =>
{
    options.DefaultExpirationTimeSpan = TimeSpan.FromDays(1);
    options.AddBasePolicy(builder => builder.Tag("__all_tag__"));
    options.AddPolicy("customPolicy", MyCustomPolicy.Instance);
});

public sealed class MyCustomPolicy : IOutputCachePolicy
{
    public static readonly MyCustomPolicy Instance = new();

    private MyCustomPolicy()
    {
    }

    ValueTask IOutputCachePolicy.CacheRequestAsync(
        OutputCacheContext context, 
        CancellationToken cancellationToken)
    {
        var attemptOutputCaching = AttemptOutputCaching(context);
        context.EnableOutputCaching = true;
        context.AllowCacheLookup = attemptOutputCaching;
        context.AllowCacheStorage = attemptOutputCaching;
        context.AllowLocking = true;

        context.Tags.Add(context.HttpContext.Request.Path); // This is what we evict by..

        // Vary by any query by default
        context.CacheVaryByRules.QueryKeys = "*";

        return ValueTask.CompletedTask;
    }

    ValueTask IOutputCachePolicy.ServeFromCacheAsync
        (OutputCacheContext context, CancellationToken cancellationToken)
    {
        return ValueTask.CompletedTask;
    }

    ValueTask IOutputCachePolicy.ServeResponseAsync
        (OutputCacheContext context, CancellationToken cancellationToken)
    {
        var response = context.HttpContext.Response;

        // Verify existence of cookie headers
        if (!StringValues.IsNullOrEmpty(response.Headers.SetCookie))
        {
            context.AllowCacheStorage = false;
            return ValueTask.CompletedTask;
        }

        // Check response code
        if (response.StatusCode != StatusCodes.Status200OK 
            && response.StatusCode != StatusCodes.Status301MovedPermanently
            && response.StatusCode != StatusCodes.Status302Found
            && response.StatusCode != StatusCodes.Status404NotFound)
        {
            context.AllowCacheStorage = false;
            return ValueTask.CompletedTask;
        }

        if (!context.HttpContext.IsResponseCacheable())
        {
            context.AllowCacheStorage = false;
            return ValueTask.CompletedTask;
        }

        return ValueTask.CompletedTask;
    }

    private static bool AttemptOutputCaching(OutputCacheContext context)
    {
        // Check if the current request fulfills the requirements
        // to be cached
        var request = context.HttpContext.Request;

        // Verify the method
        if (!HttpMethods.IsGet(request.Method) && 
            !HttpMethods.IsHead(request.Method))
        {
            return false;
        }

        // Verify existence of authorization headers
        if (!StringValues.IsNullOrEmpty(request.Headers.Authorization) || 
            request.HttpContext.User?.Identity?.IsAuthenticated == true)
        {
            return false;
        }

        return true;
    }
}
Rasmus715 commented 3 months ago

@adityamandaleeka this might not be the best place to ask this, i can always make a new issue or discussion.. Is it bad practice or are there any performance implications to having a tag per resource?

My use case is I have OutputCaching enabled in YARP and need ways of clearing single pages from my application. My thought was in my custom CachingPolicy I just call context.Tags.Add(context.HttpContext.Request.Path);. That way later I can evict based on paths. Do you see any issues with that?

My full policy

services.AddOutputCache(options =>
{
    options.DefaultExpirationTimeSpan = TimeSpan.FromDays(1);
    options.AddBasePolicy(builder => builder.Tag("__all_tag__"));
    options.AddPolicy("customPolicy", MyCustomPolicy.Instance);
});

public sealed class MyCustomPolicy : IOutputCachePolicy
{
    public static readonly MyCustomPolicy Instance = new();

    private MyCustomPolicy()
    {
    }

    ValueTask IOutputCachePolicy.CacheRequestAsync(
        OutputCacheContext context, 
        CancellationToken cancellationToken)
    {
        var attemptOutputCaching = AttemptOutputCaching(context);
        context.EnableOutputCaching = true;
        context.AllowCacheLookup = attemptOutputCaching;
        context.AllowCacheStorage = attemptOutputCaching;
        context.AllowLocking = true;

        context.Tags.Add(context.HttpContext.Request.Path); // This is what we evict by..

        // Vary by any query by default
        context.CacheVaryByRules.QueryKeys = "*";

        return ValueTask.CompletedTask;
    }

    ValueTask IOutputCachePolicy.ServeFromCacheAsync
        (OutputCacheContext context, CancellationToken cancellationToken)
    {
        return ValueTask.CompletedTask;
    }

    ValueTask IOutputCachePolicy.ServeResponseAsync
        (OutputCacheContext context, CancellationToken cancellationToken)
    {
        var response = context.HttpContext.Response;

        // Verify existence of cookie headers
        if (!StringValues.IsNullOrEmpty(response.Headers.SetCookie))
        {
            context.AllowCacheStorage = false;
            return ValueTask.CompletedTask;
        }

        // Check response code
        if (response.StatusCode != StatusCodes.Status200OK 
            && response.StatusCode != StatusCodes.Status301MovedPermanently
            && response.StatusCode != StatusCodes.Status302Found
            && response.StatusCode != StatusCodes.Status404NotFound)
        {
            context.AllowCacheStorage = false;
            return ValueTask.CompletedTask;
        }

        if (!context.HttpContext.IsResponseCacheable())
        {
            context.AllowCacheStorage = false;
            return ValueTask.CompletedTask;
        }

        return ValueTask.CompletedTask;
    }

    private static bool AttemptOutputCaching(OutputCacheContext context)
    {
        // Check if the current request fulfills the requirements
        // to be cached
        var request = context.HttpContext.Request;

        // Verify the method
        if (!HttpMethods.IsGet(request.Method) && 
            !HttpMethods.IsHead(request.Method))
        {
            return false;
        }

        // Verify existence of authorization headers
        if (!StringValues.IsNullOrEmpty(request.Headers.Authorization) || 
            request.HttpContext.User?.Identity?.IsAuthenticated == true)
        {
            return false;
        }

        return true;
    }
}

Good thinking! I've been looking for a similar logic but haven't found anything, that will tag a cache based on the requested resource. Looking forward to a similar out-of-the-box implementation :)