[API Proposal]: Decouple ValidateOnStart from Hosting
tekian opened this issue · 20 comments
Usage of ValidateOnStart() today is located in the Microsoft.Extensions.Hosting assembly\package. This is problematic as it greatly inflates dependency tree of packages that need to validate their options eagerly when the hosting package is not otherwise needed. The ValidateOnStart() functionality should be moved to the Microsoft.Extensions.Options package.
In order for a host to call a "Validate()" method, a new public interface needs to be added, called IValidateOnStart here. This interface will be injected by the ValidateOnStart() functionality and then obtained by a host (for the default host, in the Microsoft.Extensions.Hosting assembly) with a call to GetService<IValidateOnStart>() in order to trigger the validation during startup.
The new interface located in the Microsoft.Extensions.Options assembly:
namespace Microsoft.Extensions.Options
{
+ public interface IValidateOnStart
+ {
+ void Validate();
+ }
}The existing ValidateOnStart() extension method is located on the Microsoft.Extensions.DependencyInjection.OptionsBuilderExtensions extension class in the Microsoft.Extensions.Hosting assembly. The Microsoft.Extensions.Hosting assembly needs to forward this to the Microsoft.Extensions.Options assembly:
+[assembly: System.Runtime.CompilerServices.TypeForwardedTo(
+ typeof(Microsoft.Extensions.DependencyInjection.OptionsBuilderExtensions))]Original request
Background and motivation
Usage of ValidateOnStart today is tightly coupled with Microsoft.Extensions.Hosting package. The call registers both ValidationHostedService and instructs ValidateOnStart infrastructure to validate T-type upon startup. This is problematic as it greatly inflates dependency tree of packages that need to validate their options eagerly.
Following measures should be considered:
- Move
ValidateOnStartto Microsoft.Extensions.Options package. This decouple registration from implementation. Code that expresses the desire for eager validation doesn't involve registration of ValidationHostedService in the same call. The new API enables a cleaner separation of concern. - Incorporate
ValidationHostedServiceintoHostbootstrapping as a shared option infrastructure. It becomes part of defaultHostand doesn't need to be registered with everyValidateOnStartcall.
API Proposal
namespace Microsoft.Extensions.Options
{
public static class OptionsBuilderExtensions
{
public static OptionsBuilder<TOptions> ValidateOnStart<TOptions>(this OptionsBuilder<TOptions> builder)
where TOptions : class;
}(Shape of the API remains the same)
API Usage
(Usage of the API remains the same.)
Alternative Designs
No response
Risks
Moving types around.
Tagging subscribers to this area: @dotnet/area-extensions-options
See info in area-owners.md if you want to be subscribed.
Issue Details
Background and motivation
Usage of ValidateOnStart today is tightly coupled with Microsoft.Extensions.Hosting package. The call registers both ValidationHostedService and instructs ValidateOnStart infrastructure to validate T-type upon startup. This is problematic as it introduces cyclical dependencies and greatly inflates dependency tree of packages that need to validate their options eagerly.
Following measures should be considered:
- Move
ValidateOnStartto Microsoft.Extensions.Options package. This decouple registration from implementation. Code that expresses the desire for eager validation doesn't involve registration of ValidationHostedService in the same call. The new API enables a cleaner separation of concern. - Incorporate
ValidationHostedServiceintoHostbootstrapping as a shared option infrastructure. Implementation becomes part of defaultHostand doesnt need to be registered with everyValidateOnStart` call.
API Proposal
namespace Microsoft.Extensions.Options
{
public static class OptionsBuilderExtensions
{
public static OptionsBuilder<TOptions> ValidateOnStart<TOptions>(
this OptionsBuilder<TOptions> builder,
bool validateOnStart = true) where TOptions : class;
}(Shape of the API remains the same)
API Usage
(Usage of the API remains the same.)
Alternative Designs
No response
Risks
Moving types around.
| Author: | tekian |
|---|---|
| Assignees: | - |
| Labels: |
|
| Milestone: | - |
See #47821 (comment) for some reasons why it was originally done this way.
Maybe Hosting needs an IStartupService enumerable service that gets resolved and all of them notified on start of the Host. My suggestion in the above conversation was a hook in the DI container (to know when the container was built), but that was rejected because we can't rely on the specifics of our DI container implementation. It would have to be a feature across all DI implementations for us to take advantage of it.
@eerhardt Thanks for the context. What would be the difference between IStartupService and IHostedService? Regardless of which of the two it would use, I believe "the desire to validate options eagerly" can be safely de-coupled from "implementation that performs it". We could have AddValidateOnStart(this IServiceCollection... inside Microsoft.Extensions.Hosting package that would add the service. And ValidateOnStart(this IOptionsBuilder... would be in Microsoft.Extensions.Options.
What would be the difference between IStartupService and IHostedService?
The difference would be that it wouldn’t need to be kept alive for the entire process just to call a no-op Stop method on it during shutdown. It would just get created, notified, and disposed during Start.
@tekian can you help me understand the problem statement here? I see
This is problematic as it greatly inflates dependency tree of packages that need to validate their options eagerly.
Does this mean you are able to accomplish your goals today but you end up depending on more packages than you'd like? How does this correspond to the customer experience?
@ericstj The idea is to decouple the options validation from the hosting layer, allowing libraries (not only services) to depend only on Microsoft.Extensions.Options instead of Microsoft.Extensions.Hosting. Microsoft.Extensions.Hosting has a significant dependency footprint.
In particular, I propose to decouple ValidationHostedService from ValidateOnStart, even introduce a separate provider for eager options validation that can be registered and configured during the Host buildout. This way, consumers could replace it with their own implementations if desired.
If a library needs hosting anyways to do anything useful in an application what do you accomplish by trying to remove the dependency?
Are you trying to say you want to support someone doing eager validation without using hosting at all? If so, wouldn’t that need more API than what is proposed here. Where is the API that the user would trigger to indicate Start, or the observable state that an option would wish to be initialized?
This issue has been marked needs-author-action and may be missing some important information.
@ericstj Yes, the goal is to support libraries that are agnostic of hosting and services that don't use or support hosting. (e.g. Azure Functions SDK). From that perspective, we could look at current ValidationHostedService as a template for ValidateOnStart service that anybody could resolve from a DI and call. In Azure Functions SDK that would likely be a IFunctionInvocationFilter, in pure ASP.NET Core services IHostedService.
There are couple of ways how this could be achieved.
We could expose ValidateOnStartOptions and an extension method ValidateOnStart(this IServiceProvider...).
public sealed class ValidateOnStartOptions
{
public IDictionary<(Type optionsType, string optionsName), Action> Validators { get; }
}
public static class OptionsValidation
{
public static void ValidateOnStart(IServiceProvider serviceProvider)
{
var options = serviceProvider.GetService<IOptions<ValidateOnStartOptions>>();
EagerValidation.ValidateOnStart.Instance.Validate(options);
}
}
internal class ValidateOnStart
{
internal static readonly ValidateOnStart Instance = new();
public void Validate(IOptions<ValidateOnStartOptions> options)
{
// ...
}
}Or we could expose an API for the service itself and hide options.
public interface IValidateOnStart
{
void Validate();
}
public static class OptionsValidation
{
public static void ValidateOptions(IServiceProvider serviceProvider)
{
var validate = serviceProvider.GetRequiredService<IValidateOnStart>();
validate.Validate();
}
}
internal class ValidateOnStartOptions
{
public IDictionary<(Type optionsType, string optionsName), Action> Validators { get; }
}
internal class ValidateOnStart : IValidateOnStart
{
public void Validate()
{
// ...
}
}All of this would/could be part of Microsoft.Extensions.Options.
The hosted service in Microsoft.Extensions.Hosting would/could change to following.
internal sealed class ValidationHostedService : IHostedService
{
private readonly IValidateOnStart _validateOnStart;
public ValidationHostedService(IValidateOnStart validateOnStart)
{
_validateOnStart = validateOnStart;
}
public Task StartAsync(CancellationToken cancellationToken)
{
_validateOnStart.Validate();
return Task.CompletedTask;
}
public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask;
}Or similar for the other proposal.
@davidfowl @eerhardt what do you think about this separation of responsibility? It allows libraries to opt in their types to eager validation without depending on hosting.
One challenge this still has is that it would still require someone to register that ValidationHostedService. That could be avoided by making the host resolve the registered singleton (either IValidateOnStart or IOptions<ValidateOnStartOptions>) and call Validate, rather than adding the ValidationHostedService.
This sounds fine to me. It's less about validating on start and more about exposing an API that does the validation decoupled from hosting. Anyone can call it and hosting can too.
I like the goal and think it is worthwhile to pursue a solution to it.
I think there is a more general pattern here between this issue and #43149. The pattern I see is "I want to run some code when the service container is built". There is also #84847, which is very similar except it is asking for a hook just before the container is built.
I question whether this should be at the Hosting layer or built into DependencyInjection. Note there are other app models like Blazor WASM and MAUI that don't use Microsoft.Extensions.Hosting. MAUI has a similar concept here: https://learn.microsoft.com/en-us/dotnet/api/microsoft.maui.hosting.imauiinitializeservice.initialize, which is retrieved and notified when the application is built at startup time:
@DamianEdwards was following a similar pattern in https://github.com/aspnet/Benchmarks/pull/1836/files#r1175547227 using an IHostedService just to get a Database initializer executed when the app starts up.
In summary, I think we need a general mechanism for running code "at startup". Doing Option validation is just one of those things that needs this.
I think we don't necessary need a new abstraction for that. The list of IHostedService is already executed sequentually at the startup. As far as I know, the BackgroundServices are run in the background, and until IHostedService finish we won't accept traffic.
The platform support problem of IHostedService also includes Azure Functions. Giving that DI today is not asynchronous, I think it is easier to bring IHostedServices to the platform that miss it rather than creating another model. I might miss some context though.
I think we could have a slot for first hosted service in the list, the same way as we have for the last service today:
Something like:
if (services.Count != 0 && services[0].ImplementationType == typeof(StartupHostedService))
{
return;
}
services
.RemoveAll<StartupHostedService>()
.Insert(0, ServiceDescriptor.Singleton<IHostedService, StartupHostedService>());The startup hosted service then, could accept a list of some jobs that are executed independently in concurrent fashion. For instance with the following signature:
/// <summary>
/// Holds the initialization function, so we can pass it through <see cref="IServiceProvider"/>.
/// </summary>
public interface IStartupInitializer
{
/// <summary>
/// Short startup initialization job.
/// </summary>
/// <param name="token">Cancellation token.</param>
/// <returns>New <see cref="Task"/>.</returns>
Task InitializeAsync(CancellationToken token);
}To register such function, you would provide a timeout for such job. You could also use a function not to introduce an interface for simple cases:
AddStartupInitializer(this IServiceCollection services, Func<IServiceProvider, CancellationToken, Task> initializer);
AddStartupInitializer<T>(this IServiceCollection services) where T : class, IStartupInitializer;We could also consider adding a name for functions, so that there are no duplicate initializers.
I think such model fits nicely with the options validation model, populating database with the seed, checking DNS records at startup and so on.
So it sounds like folks are more/less OK with exposing a Startup method from options and we have some questions about how that might be plugged into Hosting.
@rafal-mz it just so happens we were already planning on adding a version of IStartupInitializer to hosting. It makes sense to have this feature use that functionality for registration. Probably we need to think about where IStartupInitializer lives and what is the gesture to register them. Do we require some explicit API call, or is it sufficient to just have one in the service collection?
If we put IStartupInitializer low enough and made consumption automatic in hosting, then we could pull off the move of the ValidateOnStart functionality without even adding any public API from Options (though we still could do that if we wanted to) or requiring explicit registration (though we could still do that if we wanted to). Perhaps this was what @tekian was implying the whole time but I was being too dense to follow. Regardless I'm with you now 😄.
I suppose the question is now -- do we split off the IStartupInitializer into a separate issue or just include it here? cc @steveharter who's starting to look at #43149.
@ericstj just making sure that the ball keeps on rolling here, from your previous message I'm not sure if you think that this is ready for review or if there is something we still want to validate via either @steveharter and/or @rafal-mz. Would you mind sharing what are the next steps here?
@steveharter will be drafting an API proposal for IStartupInitializer-functionality. Once this is done the work for Options (this issue) and DI (#43149) can proceed and depend on IStartupInitializer.
We changed the interface name from IValidateOnStart (describing when it's called) to IStartupValidator (describing what it is).
namespace Microsoft.Extensions.Options
{
public interface IStartupValidator
{
void Validate();
}
}and we acknowledge the movement of OptionsBuilderExtensions
+[assembly: System.Runtime.CompilerServices.TypeForwardedTo(
+ typeof(Microsoft.Extensions.DependencyInjection.OptionsBuilderExtensions))]