Complete Cache/CacheDependency implementation and add additional tests. (#351)

CZEMacLeod · web-flow · commit c9f45cecb882 · 2023-06-05T16:42:27.000-07:00
* Complete Cache/CacheDependency implementation and add additional tests.

* Add support for and tests for Cache.Insert(String, Object, CacheDependency)

* Fix issue when there are no cachedependencies when calculating utcLastModified

* Refactor cache insert code to make it cleaner and use a common code path to ensure ChangeMonitors are always handled.

* To support custom CacheDependency types that might not use change monitors under the surface, always call GetChangeMonitor and add it to the policy's change monitors.
diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters/Caching/Cache.cs b/src/Microsoft.AspNetCore.SystemWebAdapters/Caching/Cache.cs
@@ -45,21 +45,14 @@ public object Add(string key, object value, CacheDependency? dependencies, DateT
             Priority = Convert(priority),
             RemovedCallback = Convert(onRemoveCallback),
         };
-
-        return _cache.AddOrGetExisting(key, value, policy);
-    }
-
-    private static void AddChangeMonitors(CacheDependency? dependencies, CacheItemPolicy policy)
-    {
-        if (dependencies?.ChangeMonitors is not null)
-        {
-            policy.ChangeMonitors.Add(dependencies.GetChangeMonitor());
-        }
+        return _cache.AddOrGetExisting(key, value, policy, dependencies);
     }
 
     public object Get(string key) => _cache.Get(key);
 
-    public void Insert(string key, object value) => _cache.Set(key, value, new CacheItemPolicy());
+    public void Insert(string key, object value) => _cache.Set(key, value);
+
+    public void Insert(string key, object value, CacheDependency? dependencies) => _cache.Set(key, value, dependencies);
 
     public void Insert(string key, object value, CacheDependency? dependencies, DateTime absoluteExpiration, TimeSpan slidingExpiration)
     {
@@ -68,8 +61,7 @@ public void Insert(string key, object value, CacheDependency? dependencies, Date
             AbsoluteExpiration = Convert(absoluteExpiration),
             SlidingExpiration = slidingExpiration,
         };
-
-        _cache.Set(key, value, policy);
+        _cache.Set(key, value, policy, dependencies);
     }
 
     public void Insert(string key, object value, CacheDependency? dependencies, DateTime absoluteExpiration, TimeSpan slidingExpiration, CacheItemPriority priority, CacheItemRemovedCallback? onRemoveCallback)
@@ -81,8 +73,7 @@ public void Insert(string key, object value, CacheDependency? dependencies, Date
             Priority = Convert(priority),
             RemovedCallback = Convert(onRemoveCallback),
         };
-
-        _cache.Set(key, value, policy);
+        _cache.Set(key, value, policy, dependencies);
     }
 
     public void Insert(string key, object value, CacheDependency? dependencies, DateTime absoluteExpiration, TimeSpan slidingExpiration, CacheItemUpdateCallback onUpdateCallback)
@@ -93,8 +84,7 @@ public void Insert(string key, object value, CacheDependency? dependencies, Date
             SlidingExpiration = slidingExpiration,
             UpdateCallback = Convert(onUpdateCallback),
         };
-
-        _cache.Set(key, value, policy);
+        _cache.Set(key, value, policy, dependencies);
     }
 
     public object? Remove(string key) => _cache.Remove(key);
diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters/Caching/CacheDependency.cs b/src/Microsoft.AspNetCore.SystemWebAdapters/Caching/CacheDependency.cs
@@ -18,10 +18,7 @@ public class CacheDependency : IDisposable
     private string? uniqueId;
     private bool uniqueIdInitialized;
 
-    internal CacheDependency()
-    {
-        FinishInit();
-    }
+    protected CacheDependency() => FinishInit();
 
     public CacheDependency(string filename) : this(filename, DateTime.MaxValue) { }
 
@@ -31,9 +28,13 @@ public CacheDependency(string[] filenames) : this(filenames, null, null, DateTim
 
     public CacheDependency(string[] filenames, DateTime start) : this(filenames, null, null, start) { }
 
+    public CacheDependency(string[]? filenames, string[]? cachekeys) : this(filenames, cachekeys, null, DateTime.MaxValue) { }
+
     public CacheDependency(string[]? filenames, string[]? cachekeys, DateTime start) :
-        this(filenames, cachekeys, null, start)
-    { }
+        this(filenames, cachekeys, null, start) { }
+
+    public CacheDependency(string[]? filenames, string[]? cachekeys, CacheDependency? dependency) :
+        this(filenames, cachekeys, dependency, DateTime.MaxValue) { }
 
     public CacheDependency(
         string[]? filenames,
@@ -69,7 +70,7 @@ public CacheDependency(
     protected internal void FinishInit()
     {
         HasChanged = changeMonitors.Any(cm => cm.HasChanged && (cm.GetLastModifiedUtc() > utcStart));
-        utcLastModified = changeMonitors.Max(cm => cm.GetLastModifiedUtc());
+        utcLastModified = changeMonitors.Count==0 ? DateTime.MinValue : changeMonitors.Max(cm => cm.GetLastModifiedUtc());
         if (HasChanged)
         {
             NotifyDependencyChanged(this, EventArgs.Empty);
@@ -89,7 +90,7 @@ private class ChangeNotificationEventArgs : EventArgs
 
     protected void NotifyDependencyChanged(object sender, EventArgs e)
     {
-        if (initCompleted && DateTime.UtcNow > utcStart)
+        if (initCompleted && (utcStart == DateTime.MaxValue || DateTime.UtcNow > utcStart))
         {
             HasChanged = true;
             utcLastModified = DateTime.UtcNow;
diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters/Caching/CacheExtensions.cs b/src/Microsoft.AspNetCore.SystemWebAdapters/Caching/CacheExtensions.cs
@@ -0,0 +1,31 @@
+using System.Runtime.Caching;
+
+namespace System.Web.Caching;
+
+internal static class CacheExtensions
+{
+    public static CacheItemPolicy WithCacheDependency(this CacheItemPolicy policy, CacheDependency? dependency)
+    {
+        if (dependency is not null)
+        {
+            policy.ChangeMonitors.Add(dependency.GetChangeMonitor());
+        }
+        return policy;
+    }
+
+    public static void Set(this ObjectCache cache, string key, object value) =>
+        cache.Set(key, value, new CacheItemPolicy());
+
+    public static void Set(this ObjectCache cache, string key, object value,
+        CacheDependency? dependency) =>
+        cache.Set(key, value, new CacheItemPolicy().WithCacheDependency(dependency));
+
+    public static void Set(this ObjectCache cache, string key, object value,
+        CacheItemPolicy policy, CacheDependency? dependency) =>
+        cache.Set(key, value, policy.WithCacheDependency(dependency));
+
+    public static object AddOrGetExisting(this ObjectCache cache, string key, object value,
+        CacheItemPolicy policy, CacheDependency? dependency) =>
+        cache.AddOrGetExisting(key, value, policy.WithCacheDependency(dependency));
+
+}
diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters/Generated/Ref.Standard.cs b/src/Microsoft.AspNetCore.SystemWebAdapters/Generated/Ref.Standard.cs
@@ -662,18 +662,22 @@ public sealed partial class Cache : System.Collections.IEnumerable
         public object Get(string key) { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");}
         public System.Collections.IEnumerator GetEnumerator() { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");}
         public void Insert(string key, object value) { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");}
+        public void Insert(string key, object value, System.Web.Caching.CacheDependency dependencies) { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");}
         public void Insert(string key, object value, System.Web.Caching.CacheDependency dependencies, System.DateTime absoluteExpiration, System.TimeSpan slidingExpiration) { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");}
         public void Insert(string key, object value, System.Web.Caching.CacheDependency dependencies, System.DateTime absoluteExpiration, System.TimeSpan slidingExpiration, System.Web.Caching.CacheItemPriority priority, System.Web.Caching.CacheItemRemovedCallback onRemoveCallback) { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");}
         public void Insert(string key, object value, System.Web.Caching.CacheDependency dependencies, System.DateTime absoluteExpiration, System.TimeSpan slidingExpiration, System.Web.Caching.CacheItemUpdateCallback onUpdateCallback) { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");}
         public object Remove(string key) { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");}
     }
     public partial class CacheDependency : System.IDisposable
     {
+        protected CacheDependency() { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");}
         public CacheDependency(string filename) { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");}
         public CacheDependency(string filename, System.DateTime start) { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");}
         public CacheDependency(string[] filenames) { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");}
         public CacheDependency(string[] filenames, System.DateTime start) { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");}
+        public CacheDependency(string[] filenames, string[] cachekeys) { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");}
         public CacheDependency(string[] filenames, string[] cachekeys, System.DateTime start) { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");}
+        public CacheDependency(string[] filenames, string[] cachekeys, System.Web.Caching.CacheDependency dependency) { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");}
         public CacheDependency(string[] filenames, string[] cachekeys, System.Web.Caching.CacheDependency dependency, System.DateTime start) { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");}
         public bool HasChanged { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} }
         public System.DateTime UtcLastModified { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} }
diff --git a/test/Microsoft.AspNetCore.SystemWebAdapters.Tests/Caching/CacheTests.cs b/test/Microsoft.AspNetCore.SystemWebAdapters.Tests/Caching/CacheTests.cs
@@ -2,9 +2,11 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Collections;
+using System.Collections.Generic;
 using System.Runtime.Caching;
 using System.Threading.Tasks;
 using AutoFixture;
+using Microsoft.AspNetCore.SystemWebAdapters;
 using Moq;
 using Xunit;
 
@@ -292,4 +294,135 @@ public void InsertItem()
     }
 
     private sealed record Removal(string Key, object Item, CacheItemRemovedReason Reason);
+
+    [Fact]
+    public void InsertNoAbsoluteSlidingExpiration()
+    {
+        // Arrange
+        var memCache = new Mock<MemoryCache>(_fixture.Create<string>(), null);
+        var cache = new Cache(memCache.Object);
+        var key = _fixture.Create<string>();
+        var item = new object();
+
+        // Act
+        cache.Insert(key, item);
+
+        // Assert
+        memCache.Verify(m => m.Set(key, item, It.Is<CacheItemPolicy>(e => e.AbsoluteExpiration.Equals(Cache.NoAbsoluteExpiration) && e.SlidingExpiration.Equals(Cache.NoSlidingExpiration)), null), Times.Once);
+    }
+
+    [Fact]
+    public void InsertWithDependency()
+    {
+        // Arrange
+        var memCache = new Mock<MemoryCache>(_fixture.Create<string>(), null);
+        var cache = new Cache(memCache.Object);
+        var key = _fixture.Create<string>();
+        var item = new object();
+        var cacheDependency = new Mock<CacheDependency>();
+
+        // Act
+        cache.Insert(key, item, cacheDependency.Object);
+
+        // Assert
+        memCache.Verify(m => m.Set(key, item, It.Is<CacheItemPolicy>(e => e.AbsoluteExpiration.Equals(Cache.NoAbsoluteExpiration) && e.SlidingExpiration.Equals(Cache.NoSlidingExpiration)), null), Times.Once);
+    }
+
+    [Fact]
+    public async Task DependentFileCallback()
+    {
+        // Arrange
+        using var memCache = new MemoryCache(_fixture.Create<string>());
+        var cache = new Cache(memCache);
+        var item = new object();
+        var key = _fixture.Create<string>();
+        var updated = false;
+        var slidingExpiration = TimeSpan.FromMilliseconds(1);
+        CacheItemUpdateReason? updateReason = default;
+
+        void Callback(string key, CacheItemUpdateReason reason, out object? expensiveObject, out CacheDependency? dependency, out DateTime absoluteExpiration, out TimeSpan slidingExpiration)
+        {
+            expensiveObject = null;
+            dependency = null;
+            absoluteExpiration = Cache.NoAbsoluteExpiration;
+            slidingExpiration = TimeSpan.FromMilliseconds(5);
+
+            updated = true;
+            updateReason = reason;
+        }
+
+        var file = System.IO.Path.GetTempFileName();
+        await System.IO.File.WriteAllTextAsync(file, key);
+
+        using var cd = new CacheDependency(file);
+        // Act
+        cache.Insert(key, item, cd, Cache.NoAbsoluteExpiration, Cache.NoSlidingExpiration, Callback);
+
+        // Ensure file is updated
+        await System.IO.File.WriteAllTextAsync(file, DateTime.UtcNow.ToString("O"));
+
+        // Small delay here to ensure that the file change notification happens (may fail tests if too fast)
+        await Task.Delay(10);
+
+        // Force cleanup to initiate callbacks on current thread
+        memCache.Trim(100);
+
+        // Assert
+        Assert.True(updated);
+        Assert.Null(cache[key]);
+        Assert.Equal(CacheItemUpdateReason.DependencyChanged, updateReason);
+    }
+
+    [Fact]
+    public async Task DependentItemCallback()
+    {
+        // Arrange
+        using var memCache = new MemoryCache(_fixture.Create<string>());
+
+        var cache = new Cache(memCache);
+        var httpRuntime = new Mock<IHttpRuntime>();
+        httpRuntime.Setup(s => s.Cache).Returns(cache);
+        HttpRuntime.Current = httpRuntime.Object;
+
+        var item1 = new object();
+        var item2 = new object();
+        var key1 = _fixture.Create<string>();
+        var key2 = _fixture.Create<string>();
+        var updateReason = new Dictionary<string, CacheItemUpdateReason>();
+        var slidingExpiration = TimeSpan.FromMilliseconds(1);
+
+        void Callback(string key, CacheItemUpdateReason reason, out object? expensiveObject, out CacheDependency? dependency, out DateTime absoluteExpiration, out TimeSpan slidingExpiration)
+        {
+            expensiveObject = null;
+            dependency = null;
+            absoluteExpiration = Cache.NoAbsoluteExpiration;
+            slidingExpiration = Cache.NoSlidingExpiration;
+
+            updateReason[key] = reason;
+        }
+
+        // Act
+        cache.Insert(key1, item1, null, Cache.NoAbsoluteExpiration, slidingExpiration, Callback);
+
+        using var cd = new CacheDependency(null, new[] { key1 });
+        cache.Insert(key2, item2, cd, Cache.NoAbsoluteExpiration, Cache.NoSlidingExpiration, Callback);
+
+        Assert.Empty(updateReason);
+
+        // Ensure sliding expiration has hit
+        await Task.Delay(slidingExpiration);
+
+        // Force cleanup to initiate callbacks on current thread
+        memCache.Trim(100);
+
+        // Assert
+        Assert.Contains(key1, updateReason.Keys);
+        Assert.Contains(key2, updateReason.Keys);
+
+        Assert.Null(cache[key1]);
+        Assert.Null(cache[key2]);
+
+        Assert.Equal(CacheItemUpdateReason.Expired, updateReason[key1]);
+        Assert.Equal(CacheItemUpdateReason.DependencyChanged, updateReason[key2]);
+    }
 }
