Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[API Proposal]: Add extensions to create an OptionsBuilder with ValidateOnStart support #89263

Closed
steveharter opened this issue Jul 20, 2023 · 6 comments · Fixed by #89973
Closed
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-Extensions-DependencyInjection
Milestone

Comments

@steveharter
Copy link
Member

steveharter commented Jul 20, 2023

Background and motivation

Based on cloud native scenarios, these extensions methods were used and they make sense to add to the System.Microsoft.Extensions.Options.library.

Note that in cloud native, these methods were called "AddValidatedOptions"; here they are called "AddValidateOnStartOptions".

API Proposal

namespace namespace Microsoft.Extensions.DependencyInjection;

public static partial class OptionsServiceCollectionExtensions
{
    // Calls ValidateOnStart()
+   public static OptionsBuilder<TOptions> AddValidateOnStartOptions
+       <[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] TOptions>
+        (this IServiceCollection services,
+        string? name = null)
+        where TOptions : class
    {
        // Simple wrapper like this:
        return new OptionsBuilder<TOptions>(services, name ?? Options.DefaultName).ValidateOnStart();
    }

    // Supports IValidateOptions and call ValidateOnStart() above
+   public static OptionsBuilder<TOptions> AddValidateOnStartOptions<
+       [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] TOptions,
+       [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] TValidateOptions>(
+       this IServiceCollection services,
+       string? name = null)
+       where TOptions : class
+       where TValidateOptions : class, IValidateOptions<TOptions>
      {
         // Simple wrapper like this:
         services.AddOptions().TryAddEnumerable(ServiceDescriptor.Singleton<IValidateOptions<TOptions>, TValidateOptions>());
         return new OptionsBuilder<TOptions>(services, name ?? Options.DefaultName).ValidateOnStart();
      }

API Usage

// Before:
IHostBuilder builder = ...
builder.services
    .AddOptions<MyOptions>()
    .Bind(builder.Configuration.GetSection(MyConfigOptions.MyConfig))
    .Validate(o => o.MySetting == true)
    .ValidateOnStart();
// After:
IHostBuilder builder = ...
builder.Services
    .AddValidateOnStartOptions<MyOptions>()
    .Bind(builder.Configuration.GetSection(MyConfigOptions.MyConfig))
    .Validate(o => o.MySetting == true);

// Before:
IHostBuilder builder = ...
builder.Services
    .AddOptions<MyOptions>()
    .AddSingleton<IValidateOptions<MyOptions>>(new MyOptionsValidator(builder .Name)) // or TryAddEnumerable
    .Bind(builder.Configuration.GetSection(MyConfigOptions.MyConfig))
    .ValidateOnStart();
// After:
IHostBuilder builder = ...
builder.Services.
    .AddValidateOnStartOptions<MyOptions, MyOptionsValidator>(builder.Name))
    .Bind(builder.Configuration.GetSection(MyConfigOptions.MyConfig))

Alternative Designs

No response

Risks

No response

@steveharter steveharter added api-suggestion Early API idea and discussion, it is NOT ready for implementation area-Extensions-DependencyInjection labels Jul 20, 2023
@steveharter steveharter self-assigned this Jul 20, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jul 20, 2023
@ghost
Copy link

ghost commented Jul 20, 2023

Tagging subscribers to this area: @dotnet/area-extensions-dependencyinjection
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

Based on cloud native scenarios, these extensions methods were used and they make sense to add to the System.Microsoft.Extensions.DependencyInjection library.

Note that in cloud native, these methods were called "AddValidatedOptions"; here they are called "AddValidateOnStartOptions".

API Proposal

namespace namespace Microsoft.Extensions.DependencyInjection;

public static partial class OptionsBuilderExtensions
{
    // The one existing extension method:
    public static OptionsBuilder<TOptions> ValidateOnStart
        <[DynamicallyAccessedMembers(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.
            PublicParameterlessConstructor)] TOptions>
        (this OptionsBuilder<TOptions> optionsBuilder)
        where TOptions : class;

    // New one to call ValidateOnStart() above
+   public static OptionsBuilder<TOptions> AddValidateOnStartOptions
+       <[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] TOptions>
+        (this IServiceCollection services,
+        string? name = null)
+        where TOptions : class
    {
        // Simple wrapper like this:
        return new OptionsBuilder<TOptions>(services, name ?? Options.DefaultName).ValidateOnStart();
    }

    // New one to support IValidateOptions and call ValidateOnStart() above
+   public static OptionsBuilder<TOptions> AddValidateOnStartOptions<
+       [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] TOptions,
+       [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] TValidateOptions>(
+       this IServiceCollection services,
+       string? name = null)
+       where TOptions : class
+       where TValidateOptions : class, IValidateOptions<TOptions>
      {
         // Simple wrapper like this:
         services.AddOptions().TryAddEnumerable(ServiceDescriptor.Singleton<IValidateOptions<TOptions>, TValidateOptions>());
         return new OptionsBuilder<TOptions>(services, name ?? Options.DefaultName).ValidateOnStart();
      }

API Usage

// Before:
IHostBuilder builder = ...
builder.services
    .AddOptions<MyOptions>()
    .Bind(builder.Configuration.GetSection(MyConfigOptions.MyConfig))
    .Validate(o => o.MySetting == true)
    .ValidateOnStart();
// After:
IHostBuilder builder = ...
builder.Services
    .AddValidateOnStartOptions<MyOptions>()
    .Bind(builder.Configuration.GetSection(MyConfigOptions.MyConfig))
    .Validate(o => o.MySetting == true);

// Before:
IHostBuilder builder = ...
builder.Services
    .AddOptions<MyOptions>()
    .AddSingleton<IValidateOptions<MyOptions>>(new MyOptionsValidator(builder .Name)) // or TryAddEnumerable
    .Bind(builder.Configuration.GetSection(MyConfigOptions.MyConfig))
    .ValidateOnStart();
// After:
IHostBuilder builder = ...
builder.Services.
    .AddValidateOnStartOptions<MyOptions, MyOptionsValidator>(builder.Name))
    .Bind(builder.Configuration.GetSection(MyConfigOptions.MyConfig))

Alternative Designs

No response

Risks

No response

Author: steveharter
Assignees: steveharter
Labels:

api-suggestion, area-Extensions-DependencyInjection

Milestone: -

@steveharter
Copy link
Member Author

cc @geeknoid @tekian @rafal-mz to match similar cloud native functionality

@ericstj ericstj removed the untriaged New issue has not been triaged by the area owner label Jul 20, 2023
@ericstj ericstj added this to the 8.0.0 milestone Jul 20, 2023
@ericstj
Copy link
Member

ericstj commented Jul 20, 2023

@steveharter you mention

add to the System.Microsoft.Extensions.DependencyInjection library.

But I think we want to add to OptionsBuilderExtensions in the Microsoft.Extensions.Options library.

@steveharter steveharter added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Jul 24, 2023
@eerhardt
Copy link
Member

<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] TOptions>

Why does the proposal have DynamicallyAccessedMemberTypes.All on the TOptions? I believe it should only need DynamicallyAccessedMemberTypes.PublicParameterlessConstructor.

@terrajobst
Copy link
Member

terrajobst commented Jul 25, 2023

Video

  • Let's call it AddOptionsWithValidateOnStart() to make it clear that it does both AddOptions and calls ValidateOnStart.
namespace namespace Microsoft.Extensions.DependencyInjection;

public static partial class OptionsBuilderExtensions
{
    // The one existing extension method:
    // public static OptionsBuilder<TOptions> ValidateOnStart
    //     <[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] TOptions>
    //     (this OptionsBuilder<TOptions> optionsBuilder)
    //     where TOptions : class;

    // New one to call ValidateOnStart() above
    public static OptionsBuilder<TOptions> AddOptionsWithValidateOnStart
        <[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] TOptions>
         (this IServiceCollection services,
         string? name = null)
         where TOptions : class;

    // New one to support IValidateOptions and call ValidateOnStart() above
    public static OptionsBuilder<TOptions> AddOptionsWithValidateOnStart<
        [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] TOptions,
        [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] TValidateOptions>(
        this IServiceCollection services,
        string? name = null)
        where TOptions : class
        where TValidateOptions : class, IValidateOptions<TOptions>;
}

@terrajobst terrajobst added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Jul 25, 2023
@halter73
Copy link
Member

@steveharter I just noticed that this proposal put the new AddOptions... methods in OptionsBuilderExtensions. Shouldn't these be in OptionsServiceCollectionExtensions instead?

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Aug 3, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Aug 9, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Sep 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-Extensions-DependencyInjection
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants