dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.45k stars 10.03k forks source link

[RedisCache] Add a IServiceProvider parameter in AddStackExchangeRedisOutputCache() #57059

Open draudrau opened 3 months ago

draudrau commented 3 months ago

Background and Motivation

Since we can use a factory to instantiate the ConnexionMultiplexer, an additional feature would be to add a IServiceProvider in parameter of AddStackExchangeRedisOutputCache()

I know it can be not so obvious because of the method .Configure() that doesn't take itself a parameter for the IServiceProvider. I hope that this will not be an obstacle to the realization of this request.

Proposed API

namespace Microsoft.Extensions.DependencyInjection;

public static class StackExchangeRedisOutputCacheServiceCollectionExtensions
{
    public static IServiceCollection AddStackExchangeRedisOutputCache(this IServiceCollection services, Action<RedisOutputCacheOptions> setupAction)
    {
        ArgumentNullThrowHelper.ThrowIfNull(services);
        ArgumentNullThrowHelper.ThrowIfNull(setupAction);

        services.AddOptions();

        services.Configure(setupAction);
        services.AddSingleton<AspNetCore.OutputCaching.IOutputCacheStore, RedisOutputCacheStoreImpl>();

        return services;
    }

+    public static IServiceCollection AddStackExchangeRedisOutputCache(this IServiceCollection services, Action<IServiceProvider, RedisOutputCacheOptions> setupAction)
+    {
+        ...
+    }
}

Usage Examples

services.AddStackExchangeRedisOutputCache(
    (sp, options) =>
    {
        options.ConnectionMultiplexerFactory = () => Task.FromResult(sp.GetRequiredService<IConnectionMultiplexer>());
    }
mgravell commented 3 months ago

In my head I'm inherently associating both output-cache and distributed-cache in any changes here, since they're so close.

If this change has already been applied to distributed-cache and this request is "output cache should get that too", then: yup, it should.

If this is a new thing that doesn't currently exist for distributed-cache (and I don't think it does): interestingly, the core intent of this basically already exists in Aspire, via AddRedisClient() (which registers the multiplexer) and AddRedisDistributedCache() (which adds the cache, with ConnectionMultiplexerFactory pretty much exactly as shown (I'm guessing this isn't an accident). This probably a: highlights the suitability, and b: perhaps suggests alternative implementations, with the core of this in Aspire appearing to be a .Configure(...) (unless I'm misreading it): https://github.com/dotnet/aspire/blob/232e434c79acd2bd1221ad05a5d2dd5118018804/src/Components/Aspire.StackExchange.Redis.DistributedCaching/AspireRedisDistributedCacheExtensions.cs#L68