Add AddStackExchangeRedisCache & AddHsts overload

[ShreyasJejurkar]

Contributes to #39251

[BrennanConroy]

Can we add a smoke test that this new overload works?
Probably https://github.com/dotnet/aspnetcore/blob/c85baf8db0c72ae8e68643029d514b2e737c9fae/src/Caching/StackExchangeRedis/test/CacheServiceExtensionsTests.cs is a good location.

[BrennanConroy]

Ping @ShreyasJejurkar, this is almost done just needs a minor update!

[ShreyasJejurkar]

Hi @BrennanConroy, I will finish this tomorrow most likely. Sorry for the delay, last week I was busy with my office stuff! 🙁

[BrennanConroy]

Friendly ping.

I can finish it for you if it's still in-process by next week, deal?

[BrennanConroy]

Why did this change from Add to TryAdd? It breaks a test and is a behavioral change

[Tratcher]

Can redis be used with only default settings? I doesn't look like it. This overload shouldn't exist if settings are required.

[halter73]

How common is it to not use the AddStackExchangeRedisCache configuration callback? Is binding config to RedisCacheOptions a thing people do? Unlike AddHsts(), there's some value in just adding the redis cache services without the options configuration callback. And we have the equivalent non-callback overload in SignalR.StackExchangeRedis FWIW.

However, I do worry like @Tratcher that this could confuse the majority who don't know how to configure options without the AddStackExchangeRedisCache callback. I know we're not removing the existing overload, but even the presence of an overload without the callback does kinda imply that the services are usable without custom configuration. I guess it should be obvious that can't be the case for something like redis unless people are expecting some sort of dev instance to automatically be launched.

While I agree with @RehanSaeed's comment on the issue,

Yes, at the moment it's pretty strange to be forced to pass an empty delegate:

services.AddStackExchangeRedisCache(x => {});

Assuming this is a pretty uncommon use case, it does work exactly the same as this new overload. I think the slight strangeness in the rare case may be a fair price to pay to eliminate overloads most developers wouldn't know how to use.

Sorry @ShreyasJejurkar. We really appreciate the contributions. Don't close this yet just based on my feedback. I going to re-add the ready-for-review to the original issue. The new proposal will include only the AddStackExchangeRedisCache overload so we can discuss it Monday. We may or may not decide to reject the API.

[BrennanConroy]

API review rejected the new API see #39251 (comment). Thanks for the contribution though, sorry we couldn't take it 😃