aliostad / CacheCow

An implementation of HTTP Caching in .NET Core and 4.5.2+ for both the client and the server
MIT License
847 stars 172 forks source link

CacheCow.Server45 doesn't produce ETag for Redis storage. #187

Closed studenikin closed 7 years ago

studenikin commented 7 years ago

We have the following packages installed:

The configuration:

    public static class WebApiConfig
    {
        public static void Register(HttpConfiguration config)
        {
            config.MapHttpAttributeRoutes();

            var redisTagStore = new RedisEntityTagStore("redis1.com:6379,redis2.com:6379,password=12345", 0, TimeSpan.FromSeconds(10));

            config.MessageHandlers.Add(new CachingHandler(config, redisTagStore));
        }
    }
}

Etag is not generated when requesting controller. Although when I switch back to 4.0 version of CacheCow everything starts working OK.

aliostad commented 7 years ago

It is definitely getting generated - just created a test project. It is possible you have something else in your pipeline that removes it?

image

studenikin commented 7 years ago

I just created empty WebApi solution, and the tag doesn't work.

Have you tested against Redis store? When I'm testing against memory store it is working fine. But Redis store gives the issue.

aliostad commented 7 years ago

No I did not use Redis store. I will just do that.

aliostad commented 7 years ago

OK just ran and it returns fine. image

studenikin commented 7 years ago

That's weird, since ETag sometimes appears, but most of the time it doesn't. I debugged the sources and it seems the problem might occur in this place:

internal Func<HttpResponseMessage, HttpResponseMessage> GetCachingContinuation(HttpRequestMessage request)
{
    return response =>
    {
        if (!response.IsSuccessStatusCode) // only if successful carry on processing
            return response;

        try
        {
            var cacheKey = GenerateCacheKey(request);
            ExecuteCacheInvalidationRules(cacheKey, request, response);
            ExecuteCacheAdditionRules(cacheKey, request, response);
            return response;
        }
        catch (Exception ex)
        {
           Trace.TraceWarning("Exception in CacheCow: " + ex.ToString());
            return response;
        }

    };
}

My guess is that because the ExecuteCacheAdditionRules is not awaited, the response may be returned before the ExecuteCacheAdditionRules fills it out. This is happening in my case. I'm getting no Etag in 9 cases out of 10.

aliostad commented 7 years ago

Wow... You found a nasty bug. Thank you!!

aliostad commented 7 years ago

When I moved everything to async, one function stayed. Great spot. I checked the rest, it seems all fine.

aliostad commented 7 years ago

Fixed with v1.1.0 of 45 packages. Can you please test them? This should be fixed now. Closing the issue but can re-open if needs be.

studenikin commented 7 years ago

Tested. Works fine. Thanks!