Semi-Automatic options configuration improvements

[HaoK]

From @dougbu @davidfowl

A couple of months back, Fowler and I discussed ways to make it easier to configure options. My main concern at that time was the machinations necessary to set options based on other services – the whole IConfigureOptions<TOptions> rigmarole feels at best second class. We’re hoping you can find a germ in this worth adding sooner or later. That is, please run with something.

I wrote up the https://gist.github.com/dougbu/bc4880f0aadce9d94b7df91d53782da0 gist based on these discussions. The basic thing is semi-automatic discovery of Startup methods that configure options. These methods are all named ConfigureOptions, take a first parameter of the TOptions they set, and may have additional parameters grabbed from DI.

David and I covered a few questions but didn’t completely resolve them:

• Should the ConfigureOptions methods always be found and executed? Or, must the user call services.Configure<TOptions>() for every TOptions or perhaps a less-granular services.ConfigureOptions()?
• Must ConfigureOptions methods be within Startup? For example, the explicit configuration request might be services.ConfigureOptions(this) if you do want to use the Startup class.
• When are the ConfigureOptions methods executed and in what relative order? Granular services.Configure<TOptions>() methods might be semantically identical to placing an IConfigureOptions<TOptions> implementation into DI at that time. If supported, we’d need to decide if services.ConfigureOptions() called the methods in whatever order Reflection returns, alphabetically by TOptions name, or something else. And if UseStartup<Startup> implies ConfigureOptions() discovery, are the methods executed before or after IConfigureOptions<TOptions> implementations that ConfigureServices() adds?

About all we resolved is that what we have now isn’t great and Startup: IConfigureOptions<MvcOptions>, IConfigureOptions<RazorViewEngineOptions> wouldn’t help with service resolution (besides being butt-ugly).

[HaoK]

This is somewhat related to #222

Assuming we had something like a services.RegisterOptionsSetup<T> which would reflect on T and register all of the various IConfigureOptions that T implements. We could then just use that new api to register Startup automatically, the one difference from your gist would be that Startup would need to implement the interfaces. Or maybe we should just have a new dedicated SetupOptions class for this, and register it by convention if it exists:

public class SetupOptions : IConfigureOptions<MvcOptions>, IConfigureOptions<RazorViewEngineOptions>, IConfigureOptions<LocalizationOptions> {

        public SetupOptions(IStringLocalizer<Model> localizer)

        public void ConfigureOptions(LocalizationOptions options)
        {
            options.ResourcesPath = "Resources";
        }

        public void ConfigureOptions(MvcOptions options)
        {
            options.ModelBindingMessageProvider.ValueMustNotBeNullAccessor =
                    value => localizer["Value '{0}' appears to be null and that's not valid.", value];
        }

        public void ConfigureOptions(RazorViewEngineOptions options)
        {
            var embeddedProvider = new EmbeddedFileProvider(typeof(Model).GetTypeInfo().Assembly);

            options.FileProviders.Add(embeddedProvider);
        }
}

[dougbu]

the one difference from your gist would be that Startup would need to implement the interfaces

As mentioned in the description, there be 🐉s here. Startup is used far too early to get services from DI in its constructor.

dedicated SetupOptions class for this

That'll work though it seems odd to separate configuration from Startup so soon after our templates merged Program into Startup.

[dougbu]

BTW it's not clear any user IConfigurerOptions<TOptions> implementations are necessary if we're going to discover these methods using Reflection anyhow. I was thinking more about an extension of the Startup.Configure(...) convention but with all services available.

[davidfowl]

I like decoupling this feature from startup. It solves a bunch of problems.

[dougbu]

I like decoupling this feature from startup.

That's fine -- I just said it was "odd" (timing) 😸 But, will we require a separate class when performing service-reliant configuration?

[HaoK]

I don't think its too bad to have an optional dedicated ConfigureOptions class that configures a whole bunch of options.

   public class ConfigureOptions : IConfigureOptions<SomeOptions>, IPostConfigureOptions<OtherOptions>, IConfigureNamedOptions<NamedOptions> {
     public ConfigureOptions(IServiceA a, IServiceB b) { }

     public Configure(SomeOptions o) { o.Value = a.DoSomething() }

     public PostConfigure(OtherOptions o) { o.Value = b.DoSomething(); }

     public Configure(NamedOptions o, string name) { 
          if (name == b.ShouldConfigure(name)) 
               o.Value = b.DoSomething());
     }

Its not required, as you can still do the old services.AddSingleton<IConfigureOptions<T>, YourConfigure>()

I'm not exactly sure where the best place to plumb all of this, maybe a new line in Program.cs?

        public static IWebHost BuildWebHost(string[] args) =>
            WebHost.CreateDefaultBuilder(args)
                .UseStartup<Startup>()
                // This would just translate to a services.RegisterOptionsSetup<ConfigureOptions>
                .ConfigureOptions<ConfigureOptions>()
                .Build();

[HaoK]

ConfigureOptions<T> has been merged already, new sugar around not having to implement IConfigureOptoins to use depedencies is being tracked by #236 which has a PR open as well.