Skip to content

Commit

Permalink
normalize and test how per-entry vs global options are inherited
Browse files Browse the repository at this point in the history
  • Loading branch information
@mgravell
mgravell committed Jan 29, 2025
1 parent 138946a commit f78e41d
Show file tree
Hide file tree
Showing 3 changed files with 109 additions and 13 deletions.
19 changes: 11 additions & 8 deletions src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.L2.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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;
Expand Down
27 changes: 22 additions & 5 deletions src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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)
{
Expand Down
76 changes: 76 additions & 0 deletions test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/ServiceConstructionTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit f78e41d

Please sign in to comment.