Created a configurable memory cache

[kichalla]

@SajayAntony @martinpf @Tratcher

[sajayantony]

Ignoring the nuget.config fix :shipit:

[Tratcher]

Why??

[kichalla]

@Tratcher For Perf scenarios

[Tratcher]

:-1: Just implement the interface and add it to DI, there's no need to change all the callers.

[kichalla]

@Tratcher I initially thought of doing so but then thought that it would affect any of our framework components which depend on IMemoryCache and which I believe is not what the Perf team wants.

@SajayAntony, can you confirm? When the cache needs to be disabled, its only for the user code that needs to be disabled and not the framework code itself, right?

For example, the framework depends on IMemoryCache service in the following places and even they would be affected. https://github.com/aspnet/Mvc/blob/dev/src/Microsoft.AspNetCore.Mvc.TagHelpers/Internal/GlobbingUrlBuilder.cs#L42 https://github.com/aspnet/Mvc/blob/dev/src/Microsoft.AspNetCore.Mvc.TagHelpers/LinkTagHelper.cs#L103

[sajayantony]

@kichalla yes we only care about disabling memory cache for usercode since if that is being used in the framework then we want to make sure any issues are uncovered from framework misbehaving.

[Tratcher]

This still seems like a very strange abstraction. AppSettings.StoreInCache doesn't impact IMemoryCache.Get or Remove, just Set. Get rid of ConfigurableMemoryCache and inject AppSettings into StoreManagerController and check StoreInCache on line 56. Much less invasive.

[kichalla]

This still seems like a very strange abstraction. AppSettings.StoreInCache doesn't impact IMemoryCache.Get or Remove, just Set. Get rid of ConfigurableMemoryCache and inject AppSettings into StoreManagerController and check StoreInCache on line 56. Much less invasive.

But this seems more invasive. This kind of check needs to be made in all places where the cache is being used. Currently we hide this behind the ConfigurableMemoryCache layer.

[Tratcher]

It's actually less invasive as you only need to do it where you call Set, three places, as opposed to the 12 places you had to change IMemoryCache to your custom type.

[kichalla]

It's actually less invasive as you only need to do it where you call Set, three places, as opposed to the 12 places you had to change IMemoryCache to your custom type.

Not sure I like it. Also considering what @SajayAntony mentioned earlier about isolating cache usage to catch issues any framework related issues, then we need to avoid using caching in all (get, set, remove) the cases when it is disabled.

[Tratcher]

If you never call Set for a particular key you shouldn't ever run into issues with Get or Remove for that key. @SajayAntony?

[kichalla]

ping @SajayAntony

[sajayantony]

@Tratcher @kichalla as long as we don't change framework (MVC/EFs) caching behavior by changing the DI container I'm fine either way. The only change I think we are looking to do is disable data level caching so that we execute the entire code path on every request.

[Tratcher]

OK. @kichalla it should be enough to optionally avoid calling Set. You don't need a special cache implementation.

[kichalla]

updated @SajayAntony @Tratcher

[Tratcher]

:shipit: