OptionsBuilder<TOptions>.Configure(Action<TOptions>) Accept Nullable

[Foxtrek64]

Hi all,

Not sure entirely how to approach this, thus I'm making this a discussion first before making a proper API proposal.

Consider the following scenario:

public static IServiceCollection AddMyComponent
(
    this IServiceCollection services,
    Action<MyComponentOptions>? configure = null
)
{  
    services.AddOptions<MyComponentOptions>
        .BindConfiguration(MyComponentOptions.ConfigurationSectionName)
        .Configure(configure) // CS8604: Possible null reference
        .ValidateDataAnnotations()
        .ValidateOnStart();

    // More registrations

    return services;
}

This shows a common situation, where one might want to accept a delegate for configuring options, but then they encounter an issue with Configure(Action<TOptions>) not accepting nullable. If the warning is ignored and configure is null, an exception is thrown.

public virtual OptionsBuilder<TOptions> Configure(Action<TOptions> configureOptions)
{
    ThrowHelper.ThrowIfNull(configureOptions);

    services.AddSingleton<IConfigureOptions<TOptions>>(new ConfigureNamedOptions<TOptions>(Name, configureOptions));
      return this;
}

Currently, there are four work-arounds:

1. Don't accept null in your service collection extension. Force the caller to provide a configure action, even if that configuration action is an empty body.
2. Wrap the call to Configure() with an if check.
3. Explicitly make the configure body non-null in the method, i.e. configure ??= _ => { }; This continues to add an instance ConfigureNamedOptions() to the container with an empty body, which is not ideal.
4. Create an extension method which matches the body I made with the proposed API for Configure().

I personally went with option 4, naming it ConfigureOptional(), but I feel like this should be something native to the library.

Please do provide feedback. I'm sure there's stuff I've missed here that would provide either a better solution to this or justification for not accepting null in the first place.

I'm also curious what you guys may propose or an API surface for this.

[colejohnson66]

Why would you have a null configuration delegate?

[Foxtrek64]

I've seen it commonly used in configuration scenarios where you wish to provide the option for the consumer to configure their options instance, but you don't wish to make it required.

Typically I see it used like this, though this particular pattern (as opposed to AddOptions()) I generally see when the configuration is only needed during startup and is not being registered with the service provider.

var config = builder.Configuration.GetSection(MyConfig.ConfigurationSectionName).Get<MyConfig>();
configure?.Invoke(config);

// Do something with config

[colejohnson66]

If it’s not required, the traditional way is to have two overloads: one taking the delegate, and one taking nothing.

[Foxtrek64]

Allowing the null configuration delegate in Configure() would allow for DRY code. In my particular case, and I'm sure many others, the only difference between these two methods is whether or not we configure the options. Adding an empty body configuration delegate just adds expense without any additional benefits.

I suppose I could just attempt to rebuild the contents of the configure method, like this:

public static IServiceCollection AddMyComponent
(
    this IServiceCollection services,
    Action<MyComponentOptions>? configure = null
)
{  
    services.AddOptions<MyComponentOptions>()
        .BindConfiguration(MyComponentOptions.ConfigurationSectionName)
        .ValidateDataAnnotations()
        .ValidateOnStart();
 
    if (configure is not null)
    {
        services.AddSingleton<IConfigurationOptions<MyComponentOptions>(new ConfigureNamedOptions<MyComponentOptions>(Options.DefaultName, configureOptions);

    // More registrations

    return services;
}

But this feels rather unwieldy. I think it'd be better to just update the existing method to do nothing if the configure delegate is null rather than throw, but this would of course be a breaking change.