diff --git a/src/System.Linq.Dynamic.Core/Util/Cache/CacheConfig.cs b/src/System.Linq.Dynamic.Core/Util/Cache/CacheConfig.cs index 66aeb74f..8bf46fdb 100644 --- a/src/System.Linq.Dynamic.Core/Util/Cache/CacheConfig.cs +++ b/src/System.Linq.Dynamic.Core/Util/Cache/CacheConfig.cs @@ -24,4 +24,13 @@ public class CacheConfig /// By default, cleanup occurs every 10 minutes. /// public TimeSpan CleanupFrequency { get; set; } = SlidingCacheConstants.DefaultCleanupFrequency; + + /// + /// Enables returning expired cache items in scenarios where cleanup, running on a separate thread, + /// has not yet removed them. This allows for the retrieval of an expired object without needing to + /// clear and recreate it if a request is made concurrently with cleanup. Particularly useful + /// when cached items are deterministic, ensuring consistent results even from expired entries. + /// Default true; + /// + public bool ReturnExpiredItems { get; set; } = true; } \ No newline at end of file diff --git a/src/System.Linq.Dynamic.Core/Util/Cache/SlidingCache.cs b/src/System.Linq.Dynamic.Core/Util/Cache/SlidingCache.cs index f1254293..97b77051 100644 --- a/src/System.Linq.Dynamic.Core/Util/Cache/SlidingCache.cs +++ b/src/System.Linq.Dynamic.Core/Util/Cache/SlidingCache.cs @@ -1,6 +1,7 @@ using System.Collections.Concurrent; using System.Diagnostics.CodeAnalysis; using System.Linq.Dynamic.Core.Validation; +using System.Threading; namespace System.Linq.Dynamic.Core.Util.Cache; @@ -11,7 +12,9 @@ internal class SlidingCache where TKey : notnull where TValue : no private readonly IDateTimeUtils _dateTimeProvider; private readonly Action _deleteExpiredCachedItemsDelegate; private readonly long? _minCacheItemsBeforeCleanup; + private readonly bool _returnExpiredItems; private DateTime _lastCleanupTime; + private int _cleanupLocked = 0; /// /// Sliding Thread Safe Cache @@ -26,15 +29,19 @@ internal class SlidingCache where TKey : notnull where TValue : no /// Provides the Time for the Caching object. Default will be created if not supplied. Used /// for Testing classes /// + /// If a request for an item happens to be expired, but is still + /// in known, don't expire it and return it instead. public SlidingCache( TimeSpan timeToLive, TimeSpan? cleanupFrequency = null, long? minCacheItemsBeforeCleanup = null, - IDateTimeUtils? dateTimeProvider = null) + IDateTimeUtils? dateTimeProvider = null, + bool returnExpiredItems = false) { _cache = new ConcurrentDictionary>(); TimeToLive = timeToLive; _minCacheItemsBeforeCleanup = minCacheItemsBeforeCleanup; + _returnExpiredItems = returnExpiredItems; _cleanupFrequency = cleanupFrequency ?? SlidingCacheConstants.DefaultCleanupFrequency; _deleteExpiredCachedItemsDelegate = Cleanup; _dateTimeProvider = dateTimeProvider ?? new DateTimeUtils(); @@ -58,6 +65,7 @@ public SlidingCache(CacheConfig cacheConfig, IDateTimeUtils? dateTimeProvider = TimeToLive = cacheConfig.TimeToLive; _minCacheItemsBeforeCleanup = cacheConfig.MinItemsTrigger; _cleanupFrequency = cacheConfig.CleanupFrequency; + _returnExpiredItems = cacheConfig.ReturnExpiredItems; _deleteExpiredCachedItemsDelegate = Cleanup; _dateTimeProvider = dateTimeProvider ?? new DateTimeUtils(); // To prevent a scan on first call, set the last Cleanup to the current Provider time @@ -100,20 +108,29 @@ public bool TryGetValue(TKey key, [NotNullWhen(true)] out TValue? value) { Check.NotNull(key); - CleanupIfNeeded(); - - if (_cache.TryGetValue(key, out var valueAndExpiration)) + try { - if (_dateTimeProvider.UtcNow <= valueAndExpiration.ExpirationTime) + if (_cache.TryGetValue(key, out var valueAndExpiration)) { - value = valueAndExpiration.Value; - var newExpire = _dateTimeProvider.UtcNow.Add(TimeToLive); - _cache[key] = new CacheEntry(value, newExpire); - return true; + // Permit expired returns will return the object even if was expired + // this will prevent the need to re-create the object for the caller + if (_returnExpiredItems || _dateTimeProvider.UtcNow <= valueAndExpiration.ExpirationTime) + { + value = valueAndExpiration.Value; + var newExpire = _dateTimeProvider.UtcNow.Add(TimeToLive); + _cache[key] = new CacheEntry(value, newExpire); + return true; + } + + // Remove expired item + _cache.TryRemove(key, out _); } - - // Remove expired item - _cache.TryRemove(key, out _); + } + finally + { + // If permit expired returns are enabled, + // we want to ensure the cache has a chance to get the value + CleanupIfNeeded(); } value = default; @@ -131,26 +148,41 @@ public bool Remove(TKey key) private void CleanupIfNeeded() { - if (_dateTimeProvider.UtcNow - _lastCleanupTime > _cleanupFrequency - && (_minCacheItemsBeforeCleanup == null || - _cache.Count >= - _minCacheItemsBeforeCleanup) // Only cleanup if we have a minimum number of items in the cache. - ) + // Ensure this is only executing one at a time. + if (Interlocked.CompareExchange(ref _cleanupLocked, 1, 0) != 0) + { + return; + } + + try { - // Set here, so we don't have re-entry due to large collection enumeration. - _lastCleanupTime = _dateTimeProvider.UtcNow; + if (_dateTimeProvider.UtcNow - _lastCleanupTime > _cleanupFrequency + && (_minCacheItemsBeforeCleanup == null || + _cache.Count >= + _minCacheItemsBeforeCleanup) // Only cleanup if we have a minimum number of items in the cache. + ) + { + // Set here, so we don't have re-entry due to large collection enumeration. + _lastCleanupTime = _dateTimeProvider.UtcNow; - TaskUtils.Run(_deleteExpiredCachedItemsDelegate); + TaskUtils.Run(_deleteExpiredCachedItemsDelegate); + } + } + finally + { + // Release the lock + _cleanupLocked = 0; } } private void Cleanup() { - foreach (var key in _cache.Keys) + // Enumerate the key/value - safe per docs + foreach (var keyValue in _cache) { - if (_dateTimeProvider.UtcNow > _cache[key].ExpirationTime) + if (_dateTimeProvider.UtcNow > keyValue.Value.ExpirationTime) { - _cache.TryRemove(key, out _); + _cache.TryRemove(keyValue.Key, out _); } } } diff --git a/test/System.Linq.Dynamic.Core.Tests/Util/Cache/SlidingCacheTests.cs b/test/System.Linq.Dynamic.Core.Tests/Util/Cache/SlidingCacheTests.cs index 1becd9e8..7d3da124 100644 --- a/test/System.Linq.Dynamic.Core.Tests/Util/Cache/SlidingCacheTests.cs +++ b/test/System.Linq.Dynamic.Core.Tests/Util/Cache/SlidingCacheTests.cs @@ -15,6 +15,7 @@ public class SlidingCacheTests public void SlidingCache_CacheOperations() { var dateTimeUtilsMock = new Mock(); + // Configure Mock with SetupGet since SlidingCache can be non-deterministic; don't use SetupSequence dateTimeUtilsMock.SetupGet(d => d.UtcNow).Returns(UtcNow); // Arrange @@ -46,27 +47,51 @@ public void SlidingCache_CacheOperations() public void SlidingCache_TestExpire() { var dateTimeUtilsMock = new Mock(); + // Configure Mock with SetupGet since SlidingCache can be non-deterministic; don't use SetupSequence dateTimeUtilsMock.SetupGet(d => d.UtcNow).Returns(UtcNow); + // Arrange var cache = new SlidingCache(TimeSpan.FromMinutes(10), dateTimeProvider: dateTimeUtilsMock.Object); // Act cache.AddOrUpdate(1, "one"); + // move the time forward + var newDateTime = dateTimeUtilsMock.Object.UtcNow.AddMinutes(11); + dateTimeUtilsMock.SetupGet(d => d.UtcNow).Returns(newDateTime); + + // Ensure that the element has expired + cache.TryGetValue(1, out var value).Should().BeFalse($"Expected to not find the value, but found {value}"); + } + + [Fact] + public void SlidingCache_TestReturnExpiredItems() + { + var dateTimeUtilsMock = new Mock(); + // Configure Mock with SetupGet since SlidingCache can be non-deterministic; don't use SetupSequence + dateTimeUtilsMock.SetupGet(d => d.UtcNow).Returns(UtcNow); + + // Arrange + var cache = new SlidingCache(TimeSpan.FromMinutes(10), + dateTimeProvider: dateTimeUtilsMock.Object, returnExpiredItems: true); + + // Act + cache.AddOrUpdate(1, "one"); + + // move the time forward var newDateTime = dateTimeUtilsMock.Object.UtcNow.AddMinutes(11); dateTimeUtilsMock.SetupGet(d => d.UtcNow).Returns(newDateTime); - if (cache.TryGetValue(1, out var value)) - { - Assert.True(false, $"Expected to not find the value, but found {value}"); - } + // Ensure the expired item is returned from the cache + cache.TryGetValue(1, out _).Should().BeTrue($"Expected to return expired item"); } [Fact] public void SlidingCache_TestAutoExpire() { var dateTimeUtilsMock = new Mock(); + // Configure Mock with SetupGet since SlidingCache can be non-deterministic; don't use SetupSequence dateTimeUtilsMock.SetupGet(d => d.UtcNow).Returns(UtcNow); // Arrange @@ -109,6 +134,7 @@ public void SlidingCache_TestNull() public void SlidingCache_TestMinNumberBeforeTests() { var dateTimeUtilsMock = new Mock(); + // Configure Mock with SetupGet since SlidingCache can be non-deterministic; don't use SetupSequence dateTimeUtilsMock.SetupGet(d => d.UtcNow).Returns(UtcNow); // Arrange