dotnetcore / EasyCaching

:boom: EasyCaching is an open source caching library that contains basic usages and some advanced usages of caching which can help us to handle caching more easier!
MIT License
1.92k stars 321 forks source link

FEAT: add 2nd overload for AddEasyCaching() to support IServiceProvider for setupActions #487

Closed MoienTajik closed 1 year ago

MoienTajik commented 1 year ago

Added 2nd overload for AddEasyCaching() extension method to support IServiceProvider for setupActions.

cc: @catcherwong

MoienTajik commented 1 year ago

Thanks, @catcherwong. I can see one of the tests failed here because of timeout, and probably a retry will fix that. Can we have the new versions available on NuGet today, please? (1.9.1)

catcherwong commented 1 year ago

@MoienTajik v1.9.1 is available on nuget now.

artyom-p commented 10 months ago

Hey, just installed latest nuget, trying to use that extensions but somethings wrong...

Startup is stuck inside this extension as it goes into recursion and endlessly tries to resolve EasyCachingOptions again and again. Both .AddSingleton are registering the same type factory, and the second one is overriding the first one seems like.

image

Also due to services.AddSingleton<EasyCachingOptions> setup will not start until you will manually try to resolve the options, unlike the overload without IServiceProvider.

MoienTajik commented 10 months ago

I was able to reproduce the issue you mentioned. Due to the way EasyCaching registers different implementations of IEasyCachingProvider using IEasyCachingOptionsExtension, the execution of setupAction(sp, options) will be delayed until EasyCachingOptions is manually resolved from DI, which is problematic as it occurs too late.

The first overload functions correctly because it immediately creates and registers EasyCachingOptions and its extensions, rather than using a factory (services.AddSingleton(sp => ...)).

Given the current design of how IEasyCachingOptionsExtension operates, I'm unsure of a solution to this issue. It seems this commit might need to be reverted, as it doesn't behave as intended.

@catcherwong