normalize and test how per-entry vs global options are inherited

src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.L2.cs

@@ -20,8 +20,6 @@ internal partial class DefaultHybridCache
     private const string TagKeyPrefix = "__MSFT_HCT__";
     private static readonly DistributedCacheEntryOptions _tagInvalidationEntryOptions = new() { AbsoluteExpirationRelativeToNow = TimeSpan.FromDays(MaxCacheDays) };
 
-    private static readonly TimeSpan _defaultTimeout = TimeSpan.FromHours(1);
-
     [SuppressMessage("Performance", "CA1849:Call async methods when in an async method", Justification = "Manual sync check")]
     [SuppressMessage("Usage", "VSTHRD003:Avoid awaiting foreign Tasks", Justification = "Manual sync check")]
     [SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "Explicit async exception handling")]
@@ -192,7 +190,7 @@ internal void SetL1<T>(string key, CacheItem<T> value, HybridCacheEntryOptions?
             // that actually commits the add - so: if we fault, we don't want to try
             // committing a partially configured cache entry
             ICacheEntry cacheEntry = _localCache.CreateEntry(key);
-            cacheEntry.AbsoluteExpirationRelativeToNow = options?.LocalCacheExpiration ?? _defaultLocalCacheExpiration;
+            cacheEntry.AbsoluteExpirationRelativeToNow = GetL1AbsoluteExpirationRelativeToNow(options);
             cacheEntry.Value = value;
 
             if (value.TryGetSize(out var size))
@@ -221,10 +219,10 @@ private async ValueTask WritePayloadAsync(string key, CacheItem cacheItem, Buffe
         var maxLength = HybridCachePayload.GetMaxBytes(key, cacheItem.Tags, payload.Length);
         var oversized = ArrayPool<byte>.Shared.Rent(maxLength);
 
-        var length = HybridCachePayload.Write(oversized, key, cacheItem.CreationTimestamp, options?.Expiration ?? _defaultTimeout,
+        var length = HybridCachePayload.Write(oversized, key, cacheItem.CreationTimestamp, GetL2AbsoluteExpirationRelativeToNow(options),
             HybridCachePayload.PayloadFlags.None, cacheItem.Tags, payload.AsSequence());
 
-        await SetDirectL2Async(key, new(oversized, 0, length, true), GetOptions(options), token).ConfigureAwait(false);
+        await SetDirectL2Async(key, new(oversized, 0, length, true), GetL2DistributedCacheOptions(options), token).ConfigureAwait(false);
 
         ArrayPool<byte>.Shared.Return(oversized);
     }
@@ -255,12 +253,17 @@ private void ThrowPayloadLengthExceeded(int size) // splitting the exception bit
 #if NET8_0_OR_GREATER
     [SuppressMessage("Maintainability", "CA1508:Avoid dead conditional code", Justification = "False positive from unsafe accessor")]
 #endif
-    private DistributedCacheEntryOptions GetOptions(HybridCacheEntryOptions? options)
+    private DistributedCacheEntryOptions GetL2DistributedCacheOptions(HybridCacheEntryOptions? options)
     {
         DistributedCacheEntryOptions? result = null;
-        if (options is not null && options.Expiration.HasValue && options.Expiration.GetValueOrDefault() != _defaultExpiration)
+        if (options is not null)
         {
-            result = ToDistributedCacheEntryOptions(options);
+            var expiration = GetL2AbsoluteExpirationRelativeToNow(options);
+            if (expiration != _defaultExpiration)
+            {
+                // ^^^ avoid creating unnecessary DC options objects if the expiration still matches the default
+                result = ToDistributedCacheEntryOptions(options);
+            }
         }
 
         return result ?? _defaultDistributedCacheExpiration;

src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.cs

@@ -24,6 +24,8 @@ namespace Microsoft.Extensions.Caching.Hybrid.Internal;
 [SkipLocalsInit]
 internal sealed partial class DefaultHybridCache : HybridCache
 {
+    internal const int DefaultExpirationMinutes = 5;
+
     // reserve non-printable characters from keys, to prevent potential L2 abuse
     private static readonly char[] _keyReservedCharacters = Enumerable.Range(0, 32).Select(i => (char)i).ToArray();
 
@@ -109,8 +111,8 @@ public DefaultHybridCache(IOptions<HybridCacheOptions> options, IServiceProvider
         }
 
         _defaultFlags = (defaultEntryOptions?.Flags ?? HybridCacheEntryFlags.None) | _hardFlags;
-        _defaultExpiration = defaultEntryOptions?.Expiration ?? TimeSpan.FromMinutes(5);
-        _defaultLocalCacheExpiration = defaultEntryOptions?.LocalCacheExpiration ?? TimeSpan.FromMinutes(1);
+        _defaultExpiration = defaultEntryOptions?.Expiration ?? TimeSpan.FromMinutes(DefaultExpirationMinutes);
+        _defaultLocalCacheExpiration = GetEffectiveLocalCacheExpiration(defaultEntryOptions) ?? _defaultExpiration;
         _defaultDistributedCacheExpiration = new DistributedCacheEntryOptions { AbsoluteExpirationRelativeToNow = _defaultExpiration };
 
 #if NET9_0_OR_GREATER
@@ -209,6 +211,15 @@ public override ValueTask SetAsync<T>(string key, T value, HybridCacheEntryOptio
         return new(state.ExecuteDirectAsync(value, static (state, _) => new(state), options)); // note this spans L2 write etc
     }
 
+    // exposed as internal for testability
+    internal TimeSpan GetL1AbsoluteExpirationRelativeToNow(HybridCacheEntryOptions? options) => GetEffectiveLocalCacheExpiration(options) ?? _defaultLocalCacheExpiration;
+
+    internal TimeSpan GetL2AbsoluteExpirationRelativeToNow(HybridCacheEntryOptions? options) => options?.Expiration ?? _defaultExpiration;
+
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    internal HybridCacheEntryFlags GetEffectiveFlags(HybridCacheEntryOptions? options)
+        => (options?.Flags | _hardFlags) ?? _defaultFlags;
+
     private static ValueTask<T> RunWithoutCacheAsync<TState, T>(HybridCacheEntryFlags flags, TState state,
         Func<TState, CancellationToken, ValueTask<T>> underlyingDataCallback,
         CancellationToken cancellationToken)
@@ -217,9 +228,15 @@ private static ValueTask<T> RunWithoutCacheAsync<TState, T>(HybridCacheEntryFlag
             ? underlyingDataCallback(state, cancellationToken) : default;
     }
 
-    [MethodImpl(MethodImplOptions.AggressiveInlining)]
-    private HybridCacheEntryFlags GetEffectiveFlags(HybridCacheEntryOptions? options)
-        => (options?.Flags | _hardFlags) ?? _defaultFlags;
+    private static TimeSpan? GetEffectiveLocalCacheExpiration(HybridCacheEntryOptions? options)
+    {
+        // If LocalCacheExpiration is not specified, then use option's Expiration, to keep in sync by default.
+        // Or in other words: the inheritance of "LocalCacheExpiration : Expiration" in a single object takes
+        // precedence between the inheritance between per-entry options and global options, and if a caller
+        // provides a per-entry option with *just* the Expiration specified, then that is assumed to also
+        // specify the LocalCacheExpiration.
+        return options is not null ? options.LocalCacheExpiration ?? options.Expiration : null;
+    }
 
     private bool ValidateKey(string key)
     {

test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/ServiceConstructionTests.cs

@@ -204,6 +204,82 @@ public void SubclassMemoryCacheIsNotIgnored(bool manual)
         Assert.NotNull(cache.BackendCache);
     }
 
+    [Theory]
+
+    // first 4 tests; regardless of which options objects are supplied, since nothing specified: defaults are assumed
+    [InlineData(false, null, null, null, false, null, null, null)]
+    [InlineData(true, null, null, null, false, null, null, null)]
+    [InlineData(false, null, null, null, true, null, null, null)]
+    [InlineData(true, null, null, null, true, null, null, null)]
+
+    // flags; per-item wins, without merge
+    [InlineData(false, null, null, null, true, null, null, HybridCacheEntryFlags.None)]
+    [InlineData(false, null, null, null, true, null, null, HybridCacheEntryFlags.DisableLocalCacheRead, null, null, HybridCacheEntryFlags.DisableLocalCacheRead)]
+    [InlineData(true, null, null, HybridCacheEntryFlags.None, true, null, null, HybridCacheEntryFlags.DisableLocalCacheRead, null, null, HybridCacheEntryFlags.DisableLocalCacheRead)]
+    [InlineData(true, null, null, HybridCacheEntryFlags.DisableLocalCacheWrite, true, null, null, HybridCacheEntryFlags.DisableLocalCacheRead, null, null, HybridCacheEntryFlags.DisableLocalCacheRead)]
+
+    // flags; global wins if per-item omits, or no per-item flags
+    [InlineData(true, null, null, HybridCacheEntryFlags.DisableLocalCacheWrite, true, null, null, null, null, null, HybridCacheEntryFlags.DisableLocalCacheWrite)]
+    [InlineData(true, null, null, HybridCacheEntryFlags.DisableLocalCacheWrite, false, null, null, null, null, null, HybridCacheEntryFlags.DisableLocalCacheWrite)]
+
+    // local expiration; per-item wins; expiration bleeds into local expiration (but not the other way around)
+    [InlineData(false, null, null, null, true, 42, null, null, 42, 42)]
+    [InlineData(false, null, null, null, true, 42, 43, null, 42, 43)]
+    [InlineData(false, null, null, null, true, null, 43, null, null, 43)]
+
+    // global expiration; expiration bleeds into local expiration (but not the other way around)
+    [InlineData(true, 42, null, null, false, null, null, null, 42, 42)]
+    [InlineData(true, 42, 43, null, false, null, null, null, 42, 43)]
+    [InlineData(true, null, 43, null, false, null, null, null, null, 43)]
+
+    // both expirations specified; expiration bleeds into local expiration (but not the other way around)
+    [InlineData(true, 42, 43, null, true, null, null, null, 42, 43)]
+    [InlineData(true, 42, 43, null, true, 44, null, null, 44, 44)]
+    [InlineData(true, 42, 43, null, true, 44, 45, null, 44, 45)]
+    [InlineData(true, 42, 43, null, true, null, 45, null, 42, 45)]
+
+    [System.Diagnostics.CodeAnalysis.SuppressMessage("Major Code Smell", "S107:Methods should not have too many parameters",
+        Justification = "Most pragmatic and readable way of expressing multiple scenarios.")]
+    public void VerifyCacheEntryOptionsScenarios(
+        bool defaultsSpecified, int? defaultExpiration, int? defaultLocalCacheExpiration, HybridCacheEntryFlags? defaultFlags,
+        bool perItemSpecified, int? perItemExpiration, int? perItemLocalCacheExpiration, HybridCacheEntryFlags? perItemFlags,
+        int? expectedExpiration = null, int? expectedLocalCacheExpiration = null, HybridCacheEntryFlags expectedFlags = HybridCacheEntryFlags.None)
+    {
+        expectedFlags |= HybridCacheEntryFlags.DisableDistributedCache; // hard flag because no L2 present
+
+        var services = new ServiceCollection();
+        services.AddHybridCache(options =>
+        {
+            if (defaultsSpecified)
+            {
+                options.DefaultEntryOptions = new()
+                {
+                    Expiration = defaultExpiration is null ? null : TimeSpan.FromMinutes(defaultExpiration.GetValueOrDefault()),
+                    LocalCacheExpiration = defaultLocalCacheExpiration is null ? null : TimeSpan.FromMinutes(defaultLocalCacheExpiration.GetValueOrDefault()),
+                    Flags = defaultFlags,
+                };
+            }
+        });
+
+        using ServiceProvider provider = services.BuildServiceProvider();
+        var cache = Assert.IsType<DefaultHybridCache>(provider.GetRequiredService<HybridCache>());
+
+        HybridCacheEntryOptions? itemOptions = null;
+        if (perItemSpecified)
+        {
+            itemOptions = new()
+            {
+                Expiration = perItemExpiration is null ? null : TimeSpan.FromMinutes(perItemExpiration.GetValueOrDefault()),
+                LocalCacheExpiration = perItemLocalCacheExpiration is null ? null : TimeSpan.FromMinutes(perItemLocalCacheExpiration.GetValueOrDefault()),
+                Flags = perItemFlags,
+            };
+        }
+
+        Assert.Equal(expectedFlags, cache.GetEffectiveFlags(itemOptions));
+        Assert.Equal(TimeSpan.FromMinutes(expectedExpiration ?? DefaultHybridCache.DefaultExpirationMinutes), cache.GetL2AbsoluteExpirationRelativeToNow(itemOptions));
+        Assert.Equal(TimeSpan.FromMinutes(expectedLocalCacheExpiration ?? DefaultHybridCache.DefaultExpirationMinutes), cache.GetL1AbsoluteExpirationRelativeToNow(itemOptions));
+    }
+
     private class CustomMemoryCache : MemoryCache
     {
         public CustomMemoryCache(IOptions<MemoryCacheOptions> options)