dotnet / command-line-api

Command line parsing, invocation, and rendering of terminal output.
https://github.com/dotnet/command-line-api/wiki
MIT License
3.38k stars 380 forks source link

Dependency Inject / Hosting / Middleware #1025

Open Keboo opened 4 years ago

Keboo commented 4 years ago

Currently there is a lot of disparate discussion around utilizing dependency injects, hosting, and middleware. This issue aims to consolidate that discussion.

@jonsequitur, @KathleenDollard, and @Keboo met and after some discussion wanted to get more feedback on potential directions.

Current process

For a command line invocation the process flows through several steps:

Tokenization -> Parsing -> Middleware invocation -> Model binding -> Command handler invocation

Inside of the Middleware invocation various steps occur. Though the order is configurable the following is the current default order:

internal enum MiddlewareOrderInternal
{
Startup = -4000,
ExceptionHandler = -3000,
EnvironmentVariableDirective = -2600,
ConfigureConsole = -2500,
RegisterWithDotnetSuggest = -2400,
DebugDirective = -2300,
ParseDirective = -2200,
SuggestDirective = -2000,
TypoCorrection = -1900,
VersionOption = -1200,
HelpOption = -1100,
ParseErrorReporting = 1000,
}

Within all of these steps, there are various places where dependency injection may be desirable:

Considerations

Requested feedback

Related Items:

671

918

919

974

fredrikhr commented 4 years ago

I also want to point that there are different flavours of Dependency Injection leading to fundamental differences in how DI containers work.

System.CommandLine

The invocation pipeline of System.CommandLine contains a light-weight internal Service Provider providing DI-services to the BindingContext and ModelBinder types. Its access level on InvocationContext is internal meaning that callers to UseMiddleware cannot interact with the service provider directly. There is a method AddService that can be called to register a new service type. The DI system in the command-line invocation middleware has the following characteristics:

Microsoft.Extensions.DependencyInjection

So when talking about DI in general one prominent example is the Microsoft.Extensions.DependencyInjection package (and its brothers and sisters) containing the DI-system that you use in the .NET Generic Host and which people are familiar with mostly from working with ASP.NET Core. That DI system has the following main characteristics:

The default logging and configuration systems for the .NET Generic Host (Microsoft.Extensions.Logging, Microsoft.Extensions.Configuration and Microsoft.Extensions.Options) use open generic type registrations and are usually used together with the Microsoft.Extensions.DependencyInjection system.

System.CommandLine.Hosting

In this repo, we use the System.CommandLine.Hosting package to create a sort of bridge between these two systems. The way this is done is by creating an invocation middleware, that constructs a .NET Generic Host, configures it and then runs it. While the host is running, the next middleware in the pipeline is invoked. When the next middleware completes, the Host is shutdown and disposed. The next middleware gets access to an instance of IHost that is available through a System.CommandLine.ModelBinder against the binding context in the invocation context. When resolving IHost you now have access to the DI system on the other side of the bridge (the side of the .NET Generic Host) and (by default) a Microsoft.Extensions.DependencyInjection DI-container that is purposefully disjunct from System.CommandLine. You access that IServiceProvider through the Services property on the IHost instance.

.NET Generic Host services that can be resolved by the System.CommandLine ModelBinder:

The .NET Generic Host will also get access to parts of the System.CommandLine invocation context:

Examples

Imagine the following method being a command handler that is set as the Handler of a Command:

public static async Task<int> RunCommand(IHost host, ParseResult parseResult, string myOptions, 
    CancellationToken cancelToken)
{
    var hostedParseResult = host.Services.GetRequiredService<ParseResult>();
    Console.WriteLine(ReferenceEquals(parseResult, hostedParseResult)); // Will write: True

    var loggerFactory = host.Services.GetService<ILoggerFactory>() ??
        Microsoft.Extensions.Logging.Abstractions.NullLoggerFactory.Instance;
}

Above, the values for the method parameters host, parseResult, myOptions and cancelToken are all provided by the Model Binding capabilities of System.CommandLine and its BindingContext.

The body shows interactions with the Microsoft.Extensions.DependencyInjection system.

jonsequitur commented 4 years ago

Thanks, @fredrikhr. That's a great overview.

To expand on this point in the issue intro and add some use cases:

The earlier in the process that dependency injection containers are configured may incur additional performance hits; even when the dependency are not consumed.

A pattern that we've used in a few apps that use System.CommandLine is that different subcommands lead to different hosting needs. For example, in dotnet try:

Having different DI setups per command handler has two benefits:

zpittmansf commented 4 years ago

@fredrikhr very insightful! I would like to clarify on a few points in the section regarding Microsoft.Extensions.DependencyInjection. You say:

  • Has no direct capability to populate properties/fields of a type.
  • Has no direct capability to fill out parameter values for method invocation (other than the .ctor).

Neither of these items are considered dependency injection as those actions are a product of deserialization and mapping of fields, they aren't objects that the class depends on in order to be constructed successfully. The only reason I mention this is that I believe users like the model binding characteristics but it's not to be confused with the desire of getting dependencies through a constructor so that they can, for example, create unit tests around the required framework implementations using the command line api in a well known way. I don't think anyone should misconstrue model binding and method invocation parameter values with dependency injection here.

Fantastic write up once again!

zpittmansf commented 4 years ago

@Keboo I don't see this specifically called out regarding abilities gained. In addition to what is listed, being able to associate a command line handler to a given command through a DI method would be good. I think https://github.com/dotnet/command-line-api/pull/671 has done a nice job in that effort but thinking about it more, it's not as apparent to the user that the given command already has been set for you. I think to follow the DI pattern more closely it should just be a passed at construction for clarity of what is happening.

niemyjski commented 4 years ago

I posted in an issue and was linked here and I'm also dealing with this @jonsequitur. We have hosted services that we configured based on manually parsed arguments and I was wanting to use this library to define sub commands in code and then have it parse and automatically configure hosted services dynamically. This part I've really been struggling with as I want to fail fast but I've had a hard time getting access to parsed results / parsed options objects inside of configure services (in IHostBuilder).

fredrikhr commented 4 years ago

@niemyjski For the issue you describe in https://github.com/dotnet/command-line-api/issues/556#issuecomment-695022203 I'd use the BindCommandLine() extension method for options binding.

See this Gist I made for you: https://gist.github.com/fredrikhr/c9509ff5ce2071880f987d770097c843#file-dotnetcommandlineapibindcommandline-cs-L37

Note that BindCommandLine will actually even try to use a ModelBinder from DI if available, you can see in the Gist that I add a ModelBinder<MyOptions> to DI that maps --setting to the MySetting property (Note that the names do not match here, and it still works, because I instructed the model binder on how to populate the property!)

fredrikhr commented 4 years ago

@niemyjski Generally, you should not need to access the BindingContext directly from the host builder. E.g. for options you can always do:

services.AddOptions<MyOptions>()
  .Configure<BindingContext, ParseResult>((opts, bindingContext, parseResult) =>
  {
    // Initialize options using the BindingContext and/or ParseResult
  });

which is generally what the implementation of BindCommandLine does.

That being said, there is a really hacky escape hatch for the less-than-one-percent-scenario where you really cannot live without access to the BindingContext and ParseResult from the IHostBuilder: https://github.com/dotnet/command-line-api/blob/08d0408ce80894e1c9801f05561fc7b8b583c390/src/System.CommandLine.Hosting/HostingExtensions.cs#L25

This is from the implementation of the UseHost extension method. It enables you to retrieve the InvocationContext (which has access to BindingContext, ParseResult, IConsole, etc.) from the IHostBuilder properties like so:

var invocation = (InvocationContext)(hostBuilder.Properties[typeof(InvocationContext)]);
niemyjski commented 3 years ago

Thanks, I'll see I can get it to work from this. I wasn't sure I had access to the host builder from inside configure services.

jonsequitur commented 3 years ago

That being said, there is a really hacky escape hatch for the less-than-one-percent-scenario where you really cannot live without access to the BindingContext and ParseResult from the IHostBuilder:

Maybe the scenario isn't that rare and this API should be a little easier to access.

reduckted commented 3 years ago

The next middleware gets access to an instance of IHost that is available through a System.CommandLine.ModelBinder against the binding context in the invocation context.

A convenient way to get access to the IHost (or any other service) from within a middleware function would be nice to have. I'm currently using this extension method to get the IHost:

public T GetService<T>(this InvocationContext context) {
    return (T)(new ModelBinder(typeof(T)).CreateInstance(context.BindingContext));
}

I assume that's the correct way to get a service within a middleware function, but it would be nice if that was something that was already available. Even just exposing an IServiceProvider from the InvocationContext might be good enough, though that could be a bit confusing if you're using the System.CommandLine.Hosting package since it's not the same as the IServiceProvider that's exposed off the IHost.

Maybe the System.CommandLine.Hosting package could have an InvocationContext.GetHost() extension method, which would allow you to do this:

context.GetHost().Services.GetRequiredService<IMyService>()
jonsequitur commented 3 years ago

Maybe the System.CommandLine.Hosting package could have an InvocationContext.GetHost() extension method, which would allow you to do this: context.GetHost().Services.GetRequiredService()

This seems like a solid approach, as it keeps the System.CommandLine service provider out of sight.

iamlothian commented 3 years ago

Maybe I'm going about this the wrong way but here is my scenario. Im not using System.CommandLine.Hosting currently.

I have a command-line application with an option for a db connection string or a default localhost value.

Currently, I have my own middleware that builds an IHost and adds it to the binding context.

commandLineBuilder.UseMiddleware(async (context, next) => {
    context.BindingContext.AddService<IHost>((_) => IoCHost.Builder(args).Build());
    await next(context);
});

I have SomeCommand with a handler that receives the IHost and requires a service ISomeServices that depend on `ISomeRepositories that depend on the db connection.

I thought that I might be able to access the resolved options from the middleware context and build a db connection from the internal IoC host to add to my IoCHost as a service. EDIT: the best i could find was context.ParseResult.RootCommandResult.OptionResult("-connection")?.GetValueOrDefault<EventStoreClient>(); using the option alias to lookup up the type.

Is this something that is already handled by System.CommandLine.Hosting

fredrikhr commented 3 years ago

@iamlothian I am having trouble understanding your issue. In the command handler you will always only have access to the services directly added via the BindingContext.AddService.

Even when using Ms.Ext.DI you'd still need to get the IServiceProvider from the Services property on the IHost instance.

Our hosting plumbing only adds the Sys.CommandLine.InvocationContext to the IHostBuilder and calls Start on the host, while monitoring the CancellationToken for the Ctrl+C event.

iamlothian commented 3 years ago

Should I move this to another issue, it seemed relevant here?

This is what I have done, to get the result of an option (which I want the command line to handle) available to the IoCHost which wires up the rest of the application.

Configure a RootCommand which has a required option ConnectionOption

namespace X.Cli.Commands
{
    public class Root : RootCommand {
        private static readonly string DEFAULT_CONNECTION_STRING = "esdb://localhost:2113?tls=false";

        public Root(string[] args) : base("EventStore Stream Reader") 
        {
            this.AddOption(ConnectionOption);
            // ... configure root commands
            this.AddCommand(/**/); 
        }

        private static Option<EventStoreClient> ConnectionOption { 
            get {
                return new Option<EventStoreClient>(
                    aliases: new[] { "--connection", "-c"  },
                    description: "The connection string used to connect to the EventStore",
                    isDefault: true,
                    parseArgument: (result) => new EventStoreClient(
                        result.Tokens.Count == 0
                            ? EventStoreClientSettings.Create(DEFAULT_CONNECTION_STRING)
                            : EventStoreClientSettings.Create(result.Tokens[0].Value)
                    )
                );
            }
        }
    }
}

TicketRepository - depends on EventStoreClient

namespace X.Infrastructure
{
    public class TicketRepository : ITicketRepository
    {
        private readonly EventStoreClient _eventStoreClient;
        public TicketRepository(EventStoreClient eventStoreClient) {
            this._eventStoreClient = eventStoreClient;
        }
        public Task<Ticket> CreateTicket(TicketCreatedEvent ticketCreatedEvent)
        {
            // use _eventStoreClient
        }
    }
}

TicketService - depends on ITicketRepository

namespace X.Cli
{
    public class TicketService : ITicketService
    {
        private readonly ITicketRepository _ticketRepository;
        public TicketService(ITicketRepository ticketRepository)
        {
            _ticketRepository = ticketRepository;
        }
        public async Task CreateTicket(string title, string description)
        {
            var e = Ticket.Create(title, description, null);
            await _ticketRepository.CreateTicket(e);
        }
       //...
    }
}

I then have a subcommand on the root like the following. I could have just bound the EventStoreClient from the option but then I'd have to also manually build the rest of the application context starting from the repository just to use the service.

namespace X.Cli.Commands
{
    public class CreateTicket : Command
    {
        public CreateTicket() : base("create", "create a new ticket")
        {
            this.Handler = CommandHandler.Create(
                async (IHost host) => await createTicket(host)
            );
        }
        private async Task createTicket(IHost host)
        {
            await host.Services.GetRequiredService<ITicketService>().CreateTicket("New Ticket","Description");
        }
    }
}

IoCHost

namespace X.Cli
{
    public abstract class IoCHost
    {
        public static IHostBuilder Builder(string[] args) =>
            Host.CreateDefaultBuilder()
                .ConfigureAppConfiguration((_, config) => config
                    .AddCommandLine(args)
                    .AddJsonFile("appsettings.json", true)
                    .Build()
                )
                .ConfigureServices((_, services) => services
                        .AddSingleton<ITicketRepository, TicketRepository>()
                        .AddSingleton<ITicketService, TicketService>()
                        //...
                );
    }
}

Main, awkwardly mashing IHost and BindingContext together

namespace X.Cli
{
    class Program
    {
        static async Task<int> Main(string[] args) {
            var commandLineBuilder = new CommandLineBuilder(new Root(args));
            commandLineBuilder.UseMiddleware(async (context, next) => {
                context.BindingContext.AddService<IHost>((_) => 
                    IoCHost.Builder(args)
                        .ConfigureServices((_, services) => 
                            services.AddSingleton<EventStoreClient>(
                                context.ParseResult.RootCommandResult
                                    .OptionResult("--connection")
                                    ?.GetValueOrDefault<EventStoreClient>()
                            )
                        ).Build()
                );
                await next(context);
            });
            commandLineBuilder.UseDefaults();
            Parser parser = commandLineBuilder.Build();
            return await parser.InvokeAsync(args);
        }
    }
}