couchbaselabs / Couchbase.Extensions

Extensions for Couchbase for .Net Core
Apache License 2.0
27 stars 14 forks source link

CouchbaseCache modifies meta.expiration value of a document during cache read operation #115

Open sonobo opened 5 months ago

sonobo commented 5 months ago

CouchbaseCache class that implements standard IDistributedCache interface overwrites meta.expiration on every cache read. IMO this is incorrect behavior and it leads to lots of surprises and not consistent with other popular implementation like StackExchange.Redis.

The problem comes from this explicit interface implementation method:

        async Task<byte[]?> IDistributedCache.GetAsync(string key, CancellationToken token)
        {
            token.ThrowIfCancellationRequested();
            if (key == null)
            {
                throw new ArgumentNullException(nameof(key));
            }

            var collection = await CollectionProvider.GetCollectionAsync().ConfigureAwait(false);

            try
            {
                var result = await collection.GetAndTouchAsync(key, Options.LifeSpan.GetValueOrDefault(),
                        new GetAndTouchOptions().Transcoder(_transcoder))
                    .ConfigureAwait(false);
                return result.ContentAs<byte[]>();
            }
            catch (DocumentNotFoundException)
            {
                return null;
            }
        }

There's no obvious explanation of why collection.GetAndTouchAsync() is used instead of collection.GetAsync() plus passing Options.LifeSpan.GetValueOrDefault() simply resets the cache lifetime to 20 minutes on every cache read.

In many real use cases cache entry lifetime is selected carefully depending on what data is cached and absolute expiration datetime should be preserved.

Can we just remove explicit interface implementation and just keep GetAsync() insteadl? As far as I can see it also handled DocumentNotFoundException and returns null to indicate that there's no entity in the case which is already inline with expected behavior of IDistributedCache.GetAsync().

Here's a PR