Feedback from R9 review (#5012)

* Feedback from R9 review

* Get rid of custom implementation of validation

* Remove custom post-configurers and refactor ContextualOptionsFactory

src/Libraries/Microsoft.Extensions.Options.Contextual/ContextualOptionsServiceCollectionExtensions.cs

@@ -6,6 +6,8 @@
 using System.Threading.Tasks;
 using Microsoft.Extensions.DependencyInjection.Extensions;
 using Microsoft.Extensions.Options.Contextual;
+using Microsoft.Extensions.Options.Contextual.Internal;
+using Microsoft.Extensions.Options.Contextual.Provider;
 using Microsoft.Shared.Diagnostics;
 
 namespace Microsoft.Extensions.DependencyInjection;
@@ -25,8 +27,8 @@ public static IServiceCollection AddContextualOptions(this IServiceCollection se
         _ = Throw.IfNull(services).AddOptions();
 
         services.TryAdd(ServiceDescriptor.Singleton(typeof(IContextualOptionsFactory<>), typeof(ContextualOptionsFactory<>)));
-        services.TryAdd(ServiceDescriptor.Singleton(typeof(IContextualOptions<>), typeof(ContextualOptions<>)));
-        services.TryAdd(ServiceDescriptor.Singleton(typeof(INamedContextualOptions<>), typeof(ContextualOptions<>)));
+        services.TryAdd(ServiceDescriptor.Singleton(typeof(IContextualOptions<,>), typeof(ContextualOptions<,>)));
+        services.TryAdd(ServiceDescriptor.Singleton(typeof(INamedContextualOptions<,>), typeof(ContextualOptions<,>)));
 
         return services;
     }
@@ -54,108 +56,71 @@ public static IServiceCollection Configure<TOptions>(
     /// <returns>The value of <paramref name="services"/>.</returns>
     public static IServiceCollection Configure<TOptions>(
         this IServiceCollection services,
-        string name,
+        string? name,
         Func<IOptionsContext, CancellationToken, ValueTask<IConfigureContextualOptions<TOptions>>> loadOptions)
         where TOptions : class
         => services
             .AddContextualOptions()
             .AddSingleton<ILoadContextualOptions<TOptions>>(
                 new LoadContextualOptions<TOptions>(
-                    Throw.IfNull(name),
+                    name,
                     Throw.IfNull(loadOptions)));
 
     /// <summary>
     /// Registers an action used to configure a particular type of options.
     /// </summary>
     /// <typeparam name="TOptions">The options type to be configured.</typeparam>
     /// <param name="services">The <see cref="IServiceCollection"/> to add the services to.</param>
-    /// <param name="configureOptions">The action used to configure the options.</param>
+    /// <param name="configure">The action used to configure the options.</param>
     /// <returns>The value of <paramref name="services"/>.</returns>
-    public static IServiceCollection Configure<TOptions>(this IServiceCollection services, Action<IOptionsContext, TOptions> configureOptions)
+#pragma warning disable S3872 // Parameter names should not duplicate the names of their methods
+    public static IServiceCollection Configure<TOptions>(this IServiceCollection services, Action<IOptionsContext, TOptions> configure)
+#pragma warning restore S3872 // Parameter names should not duplicate the names of their methods
         where TOptions : class
-        => services.Configure(Options.Options.DefaultName, Throw.IfNull(configureOptions));
+        => services.Configure(Options.Options.DefaultName, Throw.IfNull(configure));
 
     /// <summary>
     /// Registers an action used to configure a particular type of options.
     /// </summary>
     /// <typeparam name="TOptions">The options type to be configured.</typeparam>
     /// <param name="services">The <see cref="IServiceCollection"/> to add the services to.</param>
     /// <param name="name">The name of the options to configure.</param>
-    /// <param name="configureOptions">The action used to configure the options.</param>
+    /// <param name="configure">The action used to configure the options.</param>
     /// <returns>The value of <paramref name="services"/>.</returns>
-    public static IServiceCollection Configure<TOptions>(this IServiceCollection services, string name, Action<IOptionsContext, TOptions> configureOptions)
+#pragma warning disable S3872 // Parameter names should not duplicate the names of their methods
+    public static IServiceCollection Configure<TOptions>(this IServiceCollection services, string? name, Action<IOptionsContext, TOptions> configure)
+#pragma warning restore S3872 // Parameter names should not duplicate the names of their methods
         where TOptions : class
     {
         return services.AddContextualOptions().AddSingleton<ILoadContextualOptions<TOptions>>(
             new LoadContextualOptions<TOptions>(
-                Throw.IfNull(name),
+                name,
                 (context, _) =>
                     new ValueTask<IConfigureContextualOptions<TOptions>>(
-                        new ConfigureContextualOptions<TOptions>(Throw.IfNull(configureOptions), Throw.IfNull(context)))));
+                        new ConfigureContextualOptions<TOptions>(Throw.IfNull(configure), Throw.IfNull(context)))));
     }
 
     /// <summary>
-    /// Registers an action used to initialize all instances of a particular type of options.
+    /// Registers an action used to configure all instances of a particular type of options.
     /// </summary>
     /// <typeparam name="TOptions">The options type to be configured.</typeparam>
     /// <param name="services">The <see cref="IServiceCollection"/> to add the services to.</param>
-    /// <param name="configureOptions">The action used to configure the options.</param>
-    /// <returns>The value of <paramref name="services"/>.</returns>
-    public static IServiceCollection PostConfigureAll<TOptions>(this IServiceCollection services, Action<IOptionsContext, TOptions> configureOptions)
-        where TOptions : class
-        => services.PostConfigure(null, Throw.IfNull(configureOptions));
-
-    /// <summary>
-    /// Registers an action used to initialize a particular type of options.
-    /// </summary>
-    /// <typeparam name="TOptions">The options type to be configured.</typeparam>
-    /// <param name="services">The <see cref="IServiceCollection"/> to add the services to.</param>
-    /// <param name="configureOptions">The action used to configure the options.</param>
+    /// <param name="loadOptions">The action used to configure the options.</param>
     /// <returns>The value of <paramref name="services"/>.</returns>
-    public static IServiceCollection PostConfigure<TOptions>(this IServiceCollection services, Action<IOptionsContext, TOptions> configureOptions)
+    public static IServiceCollection ConfigureAll<TOptions>(
+        this IServiceCollection services,
+        Func<IOptionsContext, CancellationToken, ValueTask<IConfigureContextualOptions<TOptions>>> loadOptions)
         where TOptions : class
-        => services.PostConfigure(Options.Options.DefaultName, Throw.IfNull(configureOptions));
+        => services.Configure(name: null, Throw.IfNull(loadOptions));
 
     /// <summary>
-    /// Registers an action used to initialize a particular type of options.
+    /// Registers an action used to configure all instances of a particular type of options.
     /// </summary>
     /// <typeparam name="TOptions">The options type to be configured.</typeparam>
     /// <param name="services">The <see cref="IServiceCollection"/> to add the services to.</param>
-    /// <param name="name">The name of the options instance.</param>
-    /// <param name="configureOptions">The action used to configure the options.</param>
-    /// <returns>The value of <paramref name="services"/>.</returns>
-    public static IServiceCollection PostConfigure<TOptions>(this IServiceCollection services, string? name, Action<IOptionsContext, TOptions> configureOptions)
-        where TOptions : class
-        => services
-            .AddContextualOptions()
-            .AddSingleton<IPostConfigureContextualOptions<TOptions>>(
-                new PostConfigureContextualOptions<TOptions>(name, Throw.IfNull(configureOptions)));
-
-    /// <summary>
-    /// Register a validation action for an options type.
-    /// </summary>
-    /// <typeparam name="TOptions">The options type to be validated.</typeparam>
-    /// <param name="services">The <see cref="IServiceCollection"/> to add the services to.</param>
-    /// <param name="validate">The validation function.</param>
-    /// <param name="failureMessage">The failure message to use when validation fails.</param>
+    /// <param name="configure">The action used to configure the options.</param>
     /// <returns>The value of <paramref name="services"/>.</returns>
-    public static IServiceCollection ValidateContextualOptions<TOptions>(this IServiceCollection services, Func<TOptions, bool> validate, string failureMessage)
+    public static IServiceCollection ConfigureAll<TOptions>(this IServiceCollection services, Action<IOptionsContext, TOptions> configure)
         where TOptions : class
-        => services.ValidateContextualOptions(Options.Options.DefaultName, Throw.IfNull(validate), Throw.IfNull(failureMessage));
-
-    /// <summary>
-    /// Register a validation action for an options type.
-    /// </summary>
-    /// <typeparam name="TOptions">The options type to be validated.</typeparam>
-    /// <param name="services">The <see cref="IServiceCollection"/> to add the services to.</param>
-    /// <param name="name">The name of the options instance.</param>
-    /// <param name="validate">The validation function.</param>
-    /// <param name="failureMessage">The failure message to use when validation fails.</param>
-    /// <returns>The value of <paramref name="services"/>.</returns>
-    public static IServiceCollection ValidateContextualOptions<TOptions>(this IServiceCollection services, string name, Func<TOptions, bool> validate, string failureMessage)
-        where TOptions : class
-        => services
-            .AddContextualOptions()
-            .AddSingleton<IValidateContextualOptions<TOptions>>(
-                new ValidateContextualOptions<TOptions>(Throw.IfNull(name), Throw.IfNull(validate), Throw.IfNull(failureMessage)));
+        => services.Configure(name: null, Throw.IfNull(configure));
 }

src/Libraries/Microsoft.Extensions.Options.Contextual/IContextualOptions.cs

@@ -10,16 +10,16 @@ namespace Microsoft.Extensions.Options.Contextual;
 /// Used to retrieve configured <typeparamref name="TOptions"/> instances.
 /// </summary>
 /// <typeparam name="TOptions">The type of options being requested.</typeparam>
-public interface IContextualOptions<TOptions>
+/// <typeparam name="TContext">A type defining the context for this request.</typeparam>
+public interface IContextualOptions<TOptions, TContext>
     where TOptions : class
+    where TContext : IOptionsContext
 {
     /// <summary>
     /// Gets the configured <typeparamref name="TOptions"/> instance.
     /// </summary>
-    /// <typeparam name="TContext">A type defining the context for this request.</typeparam>
     /// <param name="context">The context that will be used to create the options.</param>
     /// <param name="cancellationToken">The token to monitor for cancellation requests.</param>
     /// <returns>A configured instance of <typeparamref name="TOptions"/>.</returns>
-    ValueTask<TOptions> GetAsync<TContext>(in TContext context, CancellationToken cancellationToken)
-        where TContext : IOptionsContext;
+    ValueTask<TOptions> GetAsync(in TContext context, CancellationToken cancellationToken);
 }

src/Libraries/Microsoft.Extensions.Options.Contextual/INamedContextualOptions.cs

@@ -10,17 +10,17 @@ namespace Microsoft.Extensions.Options.Contextual;
 /// Used to retrieve named configured <typeparamref name="TOptions"/> instances.
 /// </summary>
 /// <typeparam name="TOptions">The type of options being requested.</typeparam>
-public interface INamedContextualOptions<TOptions> : IContextualOptions<TOptions>
+/// <typeparam name="TContext">A type defining the context for this request.</typeparam>
+public interface INamedContextualOptions<TOptions, TContext> : IContextualOptions<TOptions, TContext>
     where TOptions : class
+    where TContext : IOptionsContext
 {
     /// <summary>
     /// Gets the named configured <typeparamref name="TOptions"/> instance.
     /// </summary>
-    /// <typeparam name="TContext">A type defining the context for this request.</typeparam>
     /// <param name="name">The name of the options to get.</param>
     /// <param name="context">The context that will be used to create the options.</param>
     /// <param name="cancellationToken">The token to monitor for cancellation requests.</param>
     /// <returns>A configured instance of <typeparamref name="TOptions"/>.</returns>
-    ValueTask<TOptions> GetAsync<TContext>(string name, in TContext context, CancellationToken cancellationToken)
-        where TContext : IOptionsContext;
+    ValueTask<TOptions> GetAsync(string name, in TContext context, CancellationToken cancellationToken);
 }

src/Libraries/Microsoft.Extensions.Options.Contextual/IOptionsContext.cs

@@ -1,6 +1,8 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using Microsoft.Extensions.Options.Contextual.Provider;
+
 namespace Microsoft.Extensions.Options.Contextual;
 
 /// <summary>

src/Libraries/Microsoft.Extensions.Options.Contextual/ConfigureContextualOptions.cs → src/Libraries/Microsoft.Extensions.Options.Contextual/Internal/ConfigureContextualOptions.cs

@@ -2,9 +2,10 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System;
+using Microsoft.Extensions.Options.Contextual.Provider;
 using Microsoft.Shared.Diagnostics;
 
-namespace Microsoft.Extensions.Options.Contextual;
+namespace Microsoft.Extensions.Options.Contextual.Internal;
 
 /// <summary>
 /// Configures the <typeparamref name="TOptions"/> type.

src/Libraries/Microsoft.Extensions.Options.Contextual/ContextualOptions.cs → src/Libraries/Microsoft.Extensions.Options.Contextual/Internal/ContextualOptions.cs

@@ -5,19 +5,21 @@
 using System.Threading.Tasks;
 using Microsoft.Shared.Diagnostics;
 
-namespace Microsoft.Extensions.Options.Contextual;
+namespace Microsoft.Extensions.Options.Contextual.Internal;
 
 /// <summary>
 /// Used to retrieve configured TOptions instances based on a context.
 /// </summary>
 /// <typeparam name="TOptions">The type of options being requested.</typeparam>
-internal sealed class ContextualOptions<TOptions> : INamedContextualOptions<TOptions>
+/// <typeparam name="TContext">A type defining the context for this request.</typeparam>
+internal sealed class ContextualOptions<TOptions, TContext> : INamedContextualOptions<TOptions, TContext>
     where TOptions : class
+    where TContext : notnull, IOptionsContext
 {
     private readonly IContextualOptionsFactory<TOptions> _factory;
 
     /// <summary>
-    /// Initializes a new instance of the <see cref="ContextualOptions{TOptions}"/> class.
+    /// Initializes a new instance of the <see cref="ContextualOptions{TOptions, TContext}"/> class.
     /// </summary>
     /// <param name="factory">The factory to create instances of <typeparamref name="TOptions"/> with.</param>
     public ContextualOptions(IContextualOptionsFactory<TOptions> factory)
@@ -26,12 +28,10 @@ public ContextualOptions(IContextualOptionsFactory<TOptions> factory)
     }
 
     /// <inheritdoc/>
-    public ValueTask<TOptions> GetAsync<TContext>(in TContext context, CancellationToken cancellationToken)
-        where TContext : notnull, IOptionsContext
-        => GetAsync(Microsoft.Extensions.Options.Options.DefaultName, context, cancellationToken);
+    public ValueTask<TOptions> GetAsync(in TContext context, CancellationToken cancellationToken)
+        => GetAsync(Options.DefaultName, context, cancellationToken);
 
     /// <inheritdoc/>
-    public ValueTask<TOptions> GetAsync<TContext>(string name, in TContext context, CancellationToken cancellationToken)
-        where TContext : notnull, IOptionsContext
+    public ValueTask<TOptions> GetAsync(string name, in TContext context, CancellationToken cancellationToken)
         => _factory.CreateAsync(Throw.IfNull(name), context, cancellationToken);
 }