Allow Use of IConfigureOptions Consistently Everywhere

[RehanSaeed]

Is your feature request related to a problem? Please describe.

IConfigureOptions<T> allows you to move your configuration code to a separate class. However, it cannot be used with certain API's which require us to pass an Action<T>:

IServiceCollection AddStackExchangeRedisCache(this IServiceCollection services, Action<RedisCacheOptions> setupAction)
IServiceCollection AddHsts(this IServiceCollection services, Action<HstsOptions> configureOptions)

This is not an exhaustive list, there may be others.

Describe the solution you'd like

All API's in ASP.NET Core should be consistent and allow us to configure them using IConfigureOptions<T>. The following overloads should be added:

IServiceCollection AddStackExchangeRedisCache(this IServiceCollection services)
IServiceCollection AddHsts(this IServiceCollection services)

[davidfowl]

They compose, so it can be used. Are you saying you want overloads that don’t require the action overload?

[RehanSaeed]

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

services.AddStackExchangeRedisCache(x => {});
services.AddHsts(x => {});

It would be a good idea to review all such API's to see if there are others I missed.

[ghost]

Thanks for contacting us.
We're moving this issue to the .NET 7 Planning milestone for future evaluation / consideration. Because it's not immediately obvious that this is a bug in our framework, we would like to keep this around to collect more feedback, which can later help us determine the impact of it. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

[BrennanConroy]

Today we have two common patterns for configuring options when adding built-in ASP.NET Core features.

1. Single method that takes an Action<OptionType>? and defaults to null
   

   aspnetcore/src/Components/Server/src/DependencyInjection/ComponentServiceCollectionExtensions.cs

   Line 31 in c85baf8

   public static IServerSideBlazorBuilder AddServerSideBlazor(this IServiceCollection services, Action<CircuitOptions>? configure = null)

2. Two methods, one without Action<OptionType> and the second with Action<OptionType>
   

   aspnetcore/src/Security/Authorization/Core/src/AuthorizationServiceCollectionExtensions.cs

   Line 21 in 05b2afb

   public static IServiceCollection AddAuthorizationCore(this IServiceCollection services)

   
   

   aspnetcore/src/Security/Authorization/Core/src/AuthorizationServiceCollectionExtensions.cs

   Line 47 in 05b2afb

   public static IServiceCollection AddAuthorizationCore(this IServiceCollection services, Action<AuthorizationOptions> configure)

We should decide on the preferred approach and update any areas that are currently forcing an Action<OptionType> when they don't need to be.
Example, AddCookiePolicy is fine forcing the Action<CookiePolicyOptions> because that's the whole point of the API

aspnetcore/src/Security/CookiePolicy/src/CookiePolicyServiceCollectionExtensions.cs

Line 19 in c85baf8

public static IServiceCollection AddCookiePolicy(this IServiceCollection services, Action<CookiePolicyOptions> configureOptions)

Bringing this up with API review so we can agree on the pattern we want to use before updating places like AddStackExchangeRedisCache.

[ghost]

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

• The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
• The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
• Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

[pranavkm]

API review:

• Don't change existing overloads.
• For existing APIs that are missing the overload that does not accept an options callback, we should add one. For e.g we should do this:

+ IServiceCollection AddStackExchangeRedisCache(this IServiceCollection services)
IServiceCollection AddStackExchangeRedisCache(this IServiceCollection services, Action<RedisCacheOptions> setupAction)

• For new ones, we think standardize on using the pattern of having two overloads. This should allow for new options to be introduced in a future release without deviating from the pattern and also conforms to existing patterns in the framework.

public static IServerSideBlazorBuilder AddMyFeature(this IServiceCollection services);
public static IServerSideBlazorBuilder AddMyFeature(this IServiceCollection services, Action<MyFeatureOption> configureOptions) ;

configureOptions in the above example can be allowed to be soft-allow null values (i.e. not perform null checks) for the sake of brevity of implementation.

• In the event we only need the options type to configured without adding any other services, we should standardize on Configure??? as the name for the method e.g. ConfigureMyFeatureOptions(Action<MyFeatureOptions> configureOptions) where configureOptions is non-null.

Separately we'll approve the changes proposed in this issue:

+ IServiceCollection AddStackExchangeRedisCache(this IServiceCollection services)
+ IServiceCollection AddHsts(this IServiceCollection services)

[halter73]

We decided on the PR (#40689) that the add AddHsts() overload without a callback effectively does nothing. [1] Now the new AddStackExchangeRedisCache overload has even been called into question. Not because it does nothing, but because redis cannot work without configuring custom options.

Options can be configured separately without the callback, but this is uncommon. We're worried that introducing this overload will create confusion only to save seven characters of code in the uncommon case.

+ IServiceCollection AddStackExchangeRedisCache(this IServiceCollection services)

[1]: This brings up questions about whether UseHsts should have taken the options directly similar to UseStaticFiles and let people call .Configure<HstsOptions>( on the service collection themselves if they are so inclined, but I digress.

[ghost]

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

• The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
• The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
• Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

[halter73]

API Review Notes:

Options can be configured separately without the callback, but this is uncommon. We're worried that introducing this overload will create confusion only to save seven characters of code in the uncommon case.

We are worried that adding the non-options-configuring AddStackExchangeRedisCache(this IServiceCollection services) will create too much confusion by making people think configuring options isn't necessary. If options are configured seperately, calling AddStackExchangeRedisCache(this IServiceCollection services, Action<RedisCacheOptions> configure) with _ => { } is easy enough.