-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Description
Background and motivation
When using BindConfiguration<>
on Options of the same type, downstream IOptionsMonitor<>
instances will see multiple invocations of OnChange
when the underlying configuration changes. Wouldn't this cause issues with library authors in instances in which implementors may previously register options? If the options being bound are the same object type, wouldn't the desired outcome be for only a single change callback to happen?
For example:
Say I'm loading/binding the following options within my application
class SampleOptions
{
public DateTime AsOf { get; set; }
public Guid OptionsID { get; set; }
}
and I have a service consuming these options, but also paying attention to change notifications as so
class MyBackgroundWork(IOptionsMonitor<SampleOptions> optionsAccessor, ILogger<MyBackgroundWork> logger)
: BackgroundService
{
private readonly IDisposable? _changeWatch = optionsAccessor.OnChange((opts, name)
=> logger.LogInformation("Options Have Changed!!!! (id: {id}, as of {when})", opts.OptionsID, opts.AsOf));
protected override async Task ExecuteAsync(CancellationToken stoppingToken)
{
logger.LogInformation("Background service has started with options (id: {id}, as of {when})", optionsAccessor.CurrentValue.OptionsID, optionsAccessor.CurrentValue.AsOf);
// Do work here
await Task.Delay(TimeSpan.FromDays(1), stoppingToken);
}
public override void Dispose()
{
_changeWatch?.Dispose();
base.Dispose();
}
}
And I bootstrap my application similar to:
var builder = Host.CreateApplicationBuilder(args);
// Contrived configuration source/provider that simulates configuration changes every 5 seconds
builder.Configuration.Add<TimedConfigurationSource>(null);
builder.Services
.AddOptions<SampleOptions>()
.BindConfiguration(string.Empty);
// Here I call into a library which does additional work on my provided
// options object. Without reviewing, I don't really know what it's doing
// but there is always the chance it binds on my behalf
builder.Services.AddLibraryServices<SampleOptions>();
builder.Services.AddHostedService<MyBackgroundWork>();
var app = builder.Build();
app.Run();
When the application is run I get my initial set of options as expected
info: MyBackgroundWork[0]
Background service has started with options (id: cabf66a1-f30d-4c74-aeb9-86fbfe95f229, as of 01/23/2025 14:53:11)
info: Microsoft.Hosting.Lifetime[0]
Application started. Press Ctrl+C to shut down.
info: Microsoft.Hosting.Lifetime[0]
Hosting environment: Development
info: Microsoft.Hosting.Lifetime[0]
however, each time the app configuration is reloaded I get duplicate onchange notifications
info: MyBackgroundWork[0]
Options Have Changed!!!! (id: 1a803db7-4a0c-4159-b29a-963083a69ad1, as of 01/23/2025 14:53:16)
info: MyBackgroundWork[0]
Options Have Changed!!!! (id: 1a803db7-4a0c-4159-b29a-963083a69ad1, as of 01/23/2025 14:53:16)
info: MyBackgroundWork[0]
Options Have Changed!!!! (id: 6c3ee57d-ff48-47bc-99c6-44d68f96ce76, as of 01/23/2025 14:53:21)
info: MyBackgroundWork[0]
Options Have Changed!!!! (id: 6c3ee57d-ff48-47bc-99c6-44d68f96ce76, as of 01/23/2025 14:53:21)
Come to find out that the reason for this is that the library I'm using is also binding options on my behalf:
internal static class EXT
{
public static IServiceCollection AddLibraryServices<TOptions>(this IServiceCollection services)
where TOptions : class
{
services.AddOptions<TOptions>()
.BindConfiguration(string.Empty);
// Do other work to bootstrap services specific to this library
return services;
}
}
and for better or worse may be doing so because it is registering more services that rely on those options.
From what I can tell, the reason this is happening is because the BindConfiguration
extension is registering an IOptionsChangeTokenSource as a singleton with every call to the method
which ends up registering multiple change listeners downstream.
Doesn't this break with the other options patterns in place elsewhere a little in which these methods can be called numerous times without having to worry about duplication of registration and things? For example, I can call AddOptions on the same class over and over and I'll only ever see a single instance of the various Options services registered in my DI:
What is the downside to being able to call AddOptions<>().BindConfiguration()
numerous times and expect only a single onchange call to be expected (assuming options type and maybe name are the same)?
API Proposal
Basically, just proposing changing this from using AddSingleton
to something more like TryAddSingleton
// Same as OptionsBuilder<TOptions>.BindConfiguration
public static OptionsBuilder<TOptions> SingleBindConfiguration<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] TOptions>(
this OptionsBuilder<TOptions> optionsBuilder,
string configSectionPath,
Action<BinderOptions>? configureBinder = null)
where TOptions : class
{
optionsBuilder.Configure<IConfiguration>((opts, config) =>
{
IConfiguration section = string.Equals("", configSectionPath, StringComparison.OrdinalIgnoreCase)
? config
: config.GetSection(configSectionPath);
section.Bind(opts, configureBinder);
});
// Only register a single instance (not sure how I'd take options name into account)
optionsBuilder.Services.TryAddSingleton<IOptionsChangeTokenSource<TOptions>>(sp =>
{
return new ConfigurationChangeTokenSource<TOptions>(optionsBuilder.Name, sp.GetRequiredService<IConfiguration>());
});
return optionsBuilder;
}
Borrowing from the above description, the resulting change would yield a single call despite numerous calls to BindConfiguration
API Usage
Essentially unchanged from the existing implementation
var builder = Host.CreateApplicationBuilder(args);
builder.Services.AddOptions<SampleOptions>().BindConfiguration(string.Empty);
builder.Services.AddOptions<SampleOptions>().BindConfiguration(string.Empty);
builder.Services.AddOptions<SampleOptions>().BindConfiguration(string.Empty);
builder.Services.AddOptions<SampleOptions>().BindConfiguration(string.Empty);
var app = builder.Build();
app.Run();
Alternative Designs
I suppose, alternatively one could borrow from the direction that ServiceDescription has gone and add a new extension that allows callers to depend on a different set of expectations. Something like:
public static OptionsBuilder<TOptions> TryBindConfiguration<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] TOptions>(
this OptionsBuilder<TOptions> optionsBuilder,
string configSectionPath,
Action<BinderOptions>? configureBinder = null)
where TOptions : class
{
optionsBuilder.Configure<IConfiguration>((opts, config) =>
{
IConfiguration section = string.Equals("", configSectionPath, StringComparison.OrdinalIgnoreCase)
? config
: config.GetSection(configSectionPath);
section.Bind(opts, configureBinder);
});
// Only register a single instance (not sure how I'd take options name into account)
optionsBuilder.Services.TryAddSingleton<IOptionsChangeTokenSource<TOptions>>(sp =>
{
return new ConfigurationChangeTokenSource<TOptions>(optionsBuilder.Name, sp.GetRequiredService<IConfiguration>());
});
return optionsBuilder;
}
This would follow other DI patters but deviate away from Options a bit
Risks
Seems like the risks would be if anything were actually relying on the duplicate calls should this use case be needed. Though I can't really think of a scenario where I'd want to handle the same change event more than once.
Further, this change would have to be aware of named options changes I'd assume