ZiggyCreatures / FusionCache

FusionCache is an easy to use, fast and robust hybrid cache with advanced resiliency features.
MIT License
1.65k stars 87 forks source link

[FEATURE] Better detection of incoherent `CacheName`s options #198

Closed jodydonetti closed 6 months ago

jodydonetti commented 6 months ago

Problem

With the introduction of the builder in v0.20 FusionCache got a nice way to configure the various options and components, in a very flexible way.

In one particular scenario though, it is possible to specify something incoherent: a single instance with multiple CacheNames, by using both the high level AddFusionCache("MyCache") and the WithOptions(...) methods, like this:

services.AddFusionCache("foo")
  .WithOptions(options => {
    options.CacheName = "bar";
  });

This does not make any sense of course, but it is a case that is currently not proactively detected by FusionCache, so the runtime behaviour may lead to some surprises, which is not nice.

A slightly different and more subtle way to fall into this trap is this:

services.AddFusionCache()
  .WithOptions(options => {
    options.CacheName = "bar";
  });

This may happen because the developer didn't notice the AddFusionCache(name) overload, and thought about setting the CacheName via the WithOptions(...) method.

It should be noted that, while it may seems like a valid way to specify the CacheName and so it should be supported, that is not the case because the CacheName specified via the AddFusionCache(name) is fundamental since the name is used for different things while building the FusionCache instance, like when trying to get named options via IOptions<FusionCacheOptions> and more things internally. Finally, in crucial places like these, it's normally better to have one correct way to setup things, so the official way remains AddFusionCache(name).

Solution

An InvalidOperationException exception should be thrown when trying to instantiate the cache, typically via the IFusionCacheProvider.GetCache(name) method.

A slightly different case is for registering the default cache (a FusionCache with the default name, which can be resolved by simply having an IFusionCache param in a ctor instead of having to use an IFusionCacheProvider param): in this case the exception would be thrown by .net when trying to resolve the param.

Finally, xml docs for the related methods and exception thrown should be made better to account for this scenario, and ideally they should give clear instructions about how to correctly register named caches or to avoid changing the cache name via the WithOptions(...) when using the DI/builder approach.

Thanks Alberto!

I'd like to thank @albx that allowed me to stumble upon this special case: he's been kind enough to invite me to one of his live coding events (italian language) as a guest and see him move his first steps with FusionCache live.

That has been a really nice experience, and on one hand it allowed me to validate that the various design and naming approaches I took along the years have been solid and intuitive, and on the other hand it made me discover a small niche scenario that could've been made even better (this one).

albx commented 6 months ago

Thanks Alberto! I'd like to thank @albx that allowed me to stumble upon this special case: he's been kind enough to invite me to one of his live coding events as a guest and see him move his first steps with FusionCache live.

That has been a really nice experience, and on one hand it allowed me to validate that the various design and naming approaches I took along the years have been solid and intuitive, and on the other hand it made me discover a small niche scenario that could've been made even better (this one).

My pleasure @jodydonetti. It has been a really nice and interesting experience and I'm really happy that this experience gave a little contribution to this project :)

jodydonetti commented 6 months ago

Hi all, I just released v1.0.0-preview1 🥳

Please try it out so we can spot any potential issue before the official v1.0, thanks!

jodydonetti commented 6 months ago

Hi all, I just released preview2 🥳

Please give it a try, thanks!

jodydonetti commented 6 months ago

Hi all, I've finally released v1.0 🥳

Feels a little unreal tbh, but yep: v1.0 is out.