dotnet / dotnet/aspnetcore

Update AddStackExchangeRedisCache such that a dependency from DI could be used to configure the RedisCacheOptions

Open
#65,488 2 comments 3 reactions 0 assignees View on GitHub
api-suggestion area-middleware feature-caching
Dominant language
C#
Stars
38.4k
Forks
10.9k
Avg merge
2d 5h
Merged PRs (30d)
276

Description

## Background and Motivation

Since the `.AddStackExchangeRedisCache(() => { })` became the norm and a new proposed method `.AddStackExchangeRedisCache()` [was declined](https://github.com/dotnet/aspnetcore/issues/49721#issuecomment-2809676312), I would like to propose another new extension method, one which there already is a counterpart in the Options pattern, a generic method that would allow the user to get any service they need to configure the `RedisCacheOptions` appropriately.

Let's consider a case when, say, there already is an existing connection to a Redis server in the DI (which is, of course, `IConnectionMultiplexer` instance). How would the user go about telling the [`AddStackExchangeRedisCache`](https://github.com/dotnet/aspnetcore/blob/main/src/Caching/StackExchangeRedis/src/StackExchangeRedisCacheServiceCollectionExtensions.cs#L24) method about it and that it needs to be used (re-used)?

My guess is that the current approach requires the user to write something like this:

```csharp
services.AddOptions()
.Configure((options, connectionMultiplexer) =>
{
options.ConnectionMultiplexerFactory = () => Task.FromResult(connectionMultiplexer);
});
```
right next to the aforementioned (yet the only available method):
```csharp
services.AddStackExchangeRedisCache(() => { });
```

I'll be honest, it's not very obvious that, in case the user wants to use something from the DI, they would need to call `AddOptions`. They can only either guess or would need to decompile the `AddStackExchangeRedisCache` method, which in both cases you might say is a hassle.

## Proposed API

[Proposed PR](https://github.com/dotnet/aspnetcore/pull/65475) (on hold).
The new approach makes it easier for the user to get any service they need with a familiar delegate:

```diff
public static class StackExchangeRedisCacheServiceCollectionExtensions
{
+ public static IServiceCollection AddStackExchangeRedisCache(this IServiceCollection services, Action setupAction) where TDependency : class
+ {
+ ArgumentNullThrowHelper.ThrowIfNull(services);
+ ArgumentNullThrowHelper.ThrowIfNull(setupAction);
+
+ services.AddOptions().Configure(setupAction);
+
+ services.Add(ServiceDescriptor.Singleton());
+
+ return services;
+ }
}
```

> Side note:
> It's also a shame and/or worth noting that [RedisCacheImp](https://github.com/dotnet/aspnetcore/blob/main/src/Caching/StackExchangeRedis/src/RedisCacheImpl.cs#L12) class is `internal`, which in itself is another controversial thing, as it forces the `AddStackExchangeRedisCache` **extension method** to contain business logic, instead of pure convenience for the user.
>
> However, in this issue I only bring this up since it's **worth pointing out**, rathen than it being somehow related to the topic; so the decision (if positive) whether to also make `RedisCacheImpl` `public` within the move of this review is entirely up to the reviewing team.

## Usage Examples

The new approach makes it easier for the user to get any service they need with a familiar delegate:

```csharp
services.AddStackExchangeRedisCache((options, connectionMultiplexer) =>
{
options.ConnectionMultiplexerFactory = () => Task.FromResult(connectionMultiplexer);
});
```

Granted, while it still doesn't make it more obvious that the `RedisCacheOptions` internally are going to be resolved from the DI, it at least **hints** at it! Hey, we might even see **less** of the annoying `() => { }` thingy in the future.

## Alternative Designs

This is a convenience extension method, so an alternative is to use the existing services-options-configure, which is examined above.

## Risks

I don't see any potential risks associated with this new extension method.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.