Skip to content

Commit 15ecdb2

Browse files
critical bugfix; defer HC metadata detection because of DI cycle (#60916)
fix #60894 Co-authored-by: Marc Gravell <[email protected]>
1 parent f9aa897 commit 15ecdb2

File tree

3 files changed

+43
-10
lines changed

3 files changed

+43
-10
lines changed

Diff for: src/Caching/StackExchangeRedis/src/RedisCache.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ private static RedisValue[] GetHashFields(bool getData) => getData
5353
private long _firstErrorTimeTicks;
5454
private long _previousErrorTimeTicks;
5555

56-
internal bool HybridCacheActive { get; set; }
56+
internal virtual bool IsHybridCacheActive() => false;
5757

5858
// StackExchange.Redis will also be trying to reconnect internally,
5959
// so limit how often we recreate the ConnectionMultiplexer instance
@@ -378,7 +378,7 @@ private void TryAddSuffix(IConnectionMultiplexer connection)
378378
connection.AddLibraryNameSuffix("aspnet");
379379
connection.AddLibraryNameSuffix("DC");
380380

381-
if (HybridCacheActive)
381+
if (IsHybridCacheActive())
382382
{
383383
connection.AddLibraryNameSuffix("HC");
384384
}

Diff for: src/Caching/StackExchangeRedis/src/RedisCacheImpl.cs

+7-6
Original file line numberDiff line numberDiff line change
@@ -11,19 +11,20 @@ namespace Microsoft.Extensions.Caching.StackExchangeRedis;
1111

1212
internal sealed class RedisCacheImpl : RedisCache
1313
{
14+
private readonly IServiceProvider _services;
15+
16+
internal override bool IsHybridCacheActive()
17+
=> _services.GetService<HybridCache>() is not null;
18+
1419
public RedisCacheImpl(IOptions<RedisCacheOptions> optionsAccessor, ILogger<RedisCache> logger, IServiceProvider services)
1520
: base(optionsAccessor, logger)
1621
{
17-
HybridCacheActive = IsHybridCacheDefined(services);
22+
_services = services; // important: do not check for HybridCache here due to dependency - creates a cycle
1823
}
1924

2025
public RedisCacheImpl(IOptions<RedisCacheOptions> optionsAccessor, IServiceProvider services)
2126
: base(optionsAccessor)
2227
{
23-
HybridCacheActive = IsHybridCacheDefined(services);
28+
_services = services; // important: do not check for HybridCache here due to dependency - creates a cycle
2429
}
25-
26-
// HybridCache optionally uses IDistributedCache; if we're here, then *we are* the DC
27-
private static bool IsHybridCacheDefined(IServiceProvider services)
28-
=> services.GetService<HybridCache>() is not null;
2930
}

Diff for: src/Caching/StackExchangeRedis/test/CacheServiceExtensionsTests.cs

+34-2
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,12 @@
88
using System.Threading.Tasks;
99
using Microsoft.Extensions.Caching.Distributed;
1010
using Microsoft.Extensions.Caching.Hybrid;
11+
using Microsoft.Extensions.Caching.Memory;
1112
using Microsoft.Extensions.DependencyInjection;
1213
using Microsoft.Extensions.DependencyInjection.Extensions;
1314
using Microsoft.Extensions.Logging;
1415
using Microsoft.Extensions.Logging.Abstractions;
16+
using Microsoft.Extensions.Options;
1517
using Moq;
1618
using Xunit;
1719

@@ -142,16 +144,46 @@ public void AddStackExchangeRedisCache_HybridCacheDetected(bool hybridCacheActiv
142144
services.AddStackExchangeRedisCache(options => { });
143145
if (hybridCacheActive)
144146
{
145-
services.TryAddSingleton<HybridCache>(new DummyHybridCache());
147+
services.AddMemoryCache();
148+
services.TryAddSingleton<HybridCache, DummyHybridCache>();
146149
}
147150

148151
using var provider = services.BuildServiceProvider();
149152
var cache = Assert.IsAssignableFrom<RedisCache>(provider.GetRequiredService<IDistributedCache>());
150-
Assert.Equal(hybridCacheActive, cache.HybridCacheActive);
153+
Assert.Equal(hybridCacheActive, cache.IsHybridCacheActive());
151154
}
152155

153156
sealed class DummyHybridCache : HybridCache
154157
{
158+
// emulate the layout from HybridCache in dotnet/extensions
159+
public DummyHybridCache(IOptions<HybridCacheOptions> options, IServiceProvider services)
160+
{
161+
if (services is null)
162+
{
163+
throw new ArgumentNullException(nameof(services));
164+
}
165+
166+
var l1 = services.GetRequiredService<IMemoryCache>();
167+
_ = options.Value;
168+
var logger = services.GetService<ILoggerFactory>()?.CreateLogger(typeof(HybridCache)) ?? NullLogger.Instance;
169+
// var clock = services.GetService<TimeProvider>() ?? TimeProvider.System;
170+
var l2 = services.GetService<IDistributedCache>(); // note optional
171+
172+
// ignore L2 if it is really just the same L1, wrapped
173+
// (note not just an "is" test; if someone has a custom subclass, who knows what it does?)
174+
if (l2 is not null
175+
&& l2.GetType() == typeof(MemoryDistributedCache)
176+
&& l1.GetType() == typeof(MemoryCache))
177+
{
178+
l2 = null;
179+
}
180+
181+
IHybridCacheSerializerFactory[] factories = services.GetServices<IHybridCacheSerializerFactory>().ToArray();
182+
Array.Reverse(factories);
183+
}
184+
185+
public class HybridCacheOptions { }
186+
155187
public override ValueTask<T> GetOrCreateAsync<TState, T>(string key, TState state, Func<TState, CancellationToken, ValueTask<T>> factory, HybridCacheEntryOptions options = null, IEnumerable<string> tags = null, CancellationToken cancellationToken = default)
156188
=> throw new NotSupportedException();
157189

0 commit comments

Comments
 (0)