From 202a0173421d3ccfe5656b3392a15a6a1ca8fad7 Mon Sep 17 00:00:00 2001 From: NikiforovAll Date: Sat, 6 Jun 2020 06:31:50 +0300 Subject: [PATCH 1/4] add mvp design [wip] --- .../HostingTests.cs | 34 ++++++++++- .../System.CommandLine.Hosting.Tests.csproj | 2 +- .../CommandLineExecutorService.cs | 56 +++++++++++++++++++ .../DefaultBuilder/CommandLineHost.cs | 38 +++++++++++++ .../GenericCommandLineHostBuilder.cs | 56 +++++++++++++++++++ .../GenericHostBuilderExtensions.cs | 33 +++++++++++ .../System.CommandLine.Hosting.csproj | 2 +- 7 files changed, 218 insertions(+), 3 deletions(-) create mode 100644 src/System.CommandLine.Hosting/DefaultBuilder/CommandLineExecutorService.cs create mode 100644 src/System.CommandLine.Hosting/DefaultBuilder/CommandLineHost.cs create mode 100644 src/System.CommandLine.Hosting/DefaultBuilder/GenericCommandLineHostBuilder.cs create mode 100644 src/System.CommandLine.Hosting/DefaultBuilder/GenericHostBuilderExtensions.cs diff --git a/src/System.CommandLine.Hosting.Tests/HostingTests.cs b/src/System.CommandLine.Hosting.Tests/HostingTests.cs index c827befa35..3d3779b01f 100644 --- a/src/System.CommandLine.Hosting.Tests/HostingTests.cs +++ b/src/System.CommandLine.Hosting.Tests/HostingTests.cs @@ -3,7 +3,8 @@ using System.CommandLine.Invocation; using System.CommandLine.Parsing; using System.Linq; - +using System.Threading; +using System.Threading.Tasks; using FluentAssertions; using Microsoft.Extensions.Configuration; @@ -228,6 +229,37 @@ public static void UseHost_binds_parsed_arguments_to_options() Assert.Equal(myValue, options.MyArgument); } + + [Fact] + public static async Task CommandLineHost_default() + { + //Arrange + // var args = new string[] { $"--foo", "42" }; + // MyOptions options = null; + IHost hostToBind = null; + + var rootCmd = new RootCommand(); + rootCmd.AddOption(new Option($"--foo") { Argument = new Argument() }); + rootCmd.Handler = CommandHandler.Create((host) => + { + hostToBind = host; + }); + // Act + // var tokenSource = new CancellationTokenSource(TimeSpan.FromSeconds(1)); + CancellationTokenSource tokenSource = null; + await CommandLineHost.CreateDefaultBuilder() + .ConfigureCommandLineDefaults((CommandLineBuilder builder) => + { + // TODO: it is not possible to add it like this atm. + builder.AddCommand(rootCmd); + }) + .Build() + .RunAsync(tokenSource?.Token ?? default); + // Assert + // Assert.NotNull(hostToBind); + // Assert.Equal(42, options.MyArgument); + } + private class MyOptions { public int MyArgument { get; set; } diff --git a/src/System.CommandLine.Hosting.Tests/System.CommandLine.Hosting.Tests.csproj b/src/System.CommandLine.Hosting.Tests/System.CommandLine.Hosting.Tests.csproj index 791b5c7186..01874246b4 100644 --- a/src/System.CommandLine.Hosting.Tests/System.CommandLine.Hosting.Tests.csproj +++ b/src/System.CommandLine.Hosting.Tests/System.CommandLine.Hosting.Tests.csproj @@ -15,7 +15,7 @@ - + diff --git a/src/System.CommandLine.Hosting/DefaultBuilder/CommandLineExecutorService.cs b/src/System.CommandLine.Hosting/DefaultBuilder/CommandLineExecutorService.cs new file mode 100644 index 0000000000..ac1315f13c --- /dev/null +++ b/src/System.CommandLine.Hosting/DefaultBuilder/CommandLineExecutorService.cs @@ -0,0 +1,56 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.CommandLine.Invocation; +using System.CommandLine.Parsing; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; + +namespace System.CommandLine.Hosting +{ + internal class CommandLineExecutorService : IHostedService + { + private readonly ILoggerFactory loggerFactory; + private readonly IHostApplicationLifetime appLifetime; + private readonly IHost host; + private readonly InvocationContext invocation; + private readonly ParseResult parseResult; + + public CommandLineExecutorService( + ILoggerFactory loggerFactory, + IConfiguration configuration, + IHostApplicationLifetime appLifetime, + IHost host, + InvocationContext invocation, + ParseResult parseResult) + { + this.appLifetime = appLifetime; + this.host = host; + this.invocation = invocation; + this.parseResult = parseResult; + this.loggerFactory = loggerFactory; + // Logger = loggerFactory.CreateLogger("Microsoft.AspNetCore.Hosting.Diagnostics"); + Configuration = configuration; + } + + public ILogger Logger { get; } + // Only for high level lifetime events + public IConfiguration Configuration { get; } + + public async Task StartAsync(CancellationToken cancellationToken) + { + // TODO: consider how to make more robust + invocation.BindingContext.AddService(typeof(IHost), _ => host); + await parseResult.InvokeAsync(); + appLifetime.StopApplication(); + } + + public Task StopAsync(CancellationToken cancellationToken) + { + return Task.CompletedTask; + } + } +} diff --git a/src/System.CommandLine.Hosting/DefaultBuilder/CommandLineHost.cs b/src/System.CommandLine.Hosting/DefaultBuilder/CommandLineHost.cs new file mode 100644 index 0000000000..27a9d49ec1 --- /dev/null +++ b/src/System.CommandLine.Hosting/DefaultBuilder/CommandLineHost.cs @@ -0,0 +1,38 @@ +using Microsoft.Extensions.Hosting; +using System.CommandLine.Builder; + +namespace System.CommandLine.Hosting +{ + /// + /// Provides convenience methods for creating instances of and with pre-configured defaults. + /// + public static class CommandLineHost + { + /// + /// Initializes a new instance of the with pre-configured defaults. + /// + /// + /// + /// The initialized . + public static IHostBuilder CreateDefaultBuilder() => + CreateDefaultBuilder(args: null); + + /// + /// Initializes a new instance of the with pre-configured defaults. + /// + /// + /// + /// The command line args. + /// The initialized . + public static IHostBuilder CreateDefaultBuilder(string[] args) + { + var builder = Host.CreateDefaultBuilder(args) + .ConfigureCommandLineDefaults(cmdBuilder => cmdBuilder.UseDefaults(), args); + return builder; + // TODO: add remaining args parsing + // var argsRemaining = invocation.ParseResult.UnparsedTokens.ToArray(); + // TODO: + // ConfigureCommandLineDefaults: remove redundant call GenericCommandLineHostBuilder.Configure + } + } +} diff --git a/src/System.CommandLine.Hosting/DefaultBuilder/GenericCommandLineHostBuilder.cs b/src/System.CommandLine.Hosting/DefaultBuilder/GenericCommandLineHostBuilder.cs new file mode 100644 index 0000000000..bd2ec506b6 --- /dev/null +++ b/src/System.CommandLine.Hosting/DefaultBuilder/GenericCommandLineHostBuilder.cs @@ -0,0 +1,56 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.CommandLine.Builder; +using System.CommandLine.Invocation; +using System.CommandLine.Parsing; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection.Extensions; +using Microsoft.Extensions.Hosting; + +namespace System.CommandLine.Hosting +{ + internal class GenericCommandLineHostBuilder + { + + private const string ConfigurationDirectiveName = "config"; + public string[] Args { get; } + private readonly IHostBuilder builder; + private CommandLineBuilder commandLineBuilder; + + public GenericCommandLineHostBuilder(IHostBuilder builder, string[] args = default) + { + this.builder = builder; + commandLineBuilder = new CommandLineBuilder(); + Args = args; + } + + public void Configure(Action configure) + { + configure(commandLineBuilder); + Parser parser = commandLineBuilder.Build(); + ParseResult parseResult = parser.Parse(Args); + // TODO: throw/handle error in parse result at this stage + var invocation = new InvocationContext(parseResult); + AddSystemCommandLine(builder, invocation); + builder.ConfigureHostConfiguration(config => + { + config.AddCommandLineDirectives(invocation.ParseResult, ConfigurationDirectiveName); + }); + } + + private static void AddSystemCommandLine(IHostBuilder host, InvocationContext invocation) + { + host.ConfigureServices(services => + { + services.TryAddSingleton(invocation); + services.AddSingleton(invocation.Console); + // semantically it is transient dependency + services.AddTransient(_ => invocation.InvocationResult); + services.AddTransient(_ => invocation.ParseResult); + }); + } + + } +} diff --git a/src/System.CommandLine.Hosting/DefaultBuilder/GenericHostBuilderExtensions.cs b/src/System.CommandLine.Hosting/DefaultBuilder/GenericHostBuilderExtensions.cs new file mode 100644 index 0000000000..6387d8c8e9 --- /dev/null +++ b/src/System.CommandLine.Hosting/DefaultBuilder/GenericHostBuilderExtensions.cs @@ -0,0 +1,33 @@ +using System.CommandLine.Builder; +using Microsoft.Extensions.DependencyInjection; +// using System.CommandLine.Hosting; +using Microsoft.Extensions.Hosting; + +namespace System.CommandLine.Hosting +{ + /// + /// Extension methods for configuring the IWebHostBuilder. + /// + public static class GenericHostBuilderExtensions + { + /// + /// Initializes a new instance of the class with pre-configured defaults. + /// + /// + /// + /// The instance to configure + /// The configure callback + /// The for chaining. + public static IHostBuilder ConfigureCommandLineDefaults( + this IHostBuilder builder, + Action configure, + string[] args = default) + { + var host = new GenericCommandLineHostBuilder(builder, args); + host.Configure(configure); + builder.ConfigureServices((context, services) => + services.AddHostedService()); + return builder; + } + } +} diff --git a/src/System.CommandLine.Hosting/System.CommandLine.Hosting.csproj b/src/System.CommandLine.Hosting/System.CommandLine.Hosting.csproj index 1d00cff230..5b9512f79b 100644 --- a/src/System.CommandLine.Hosting/System.CommandLine.Hosting.csproj +++ b/src/System.CommandLine.Hosting/System.CommandLine.Hosting.csproj @@ -12,7 +12,7 @@ - + From cf4e9cbbd858cd09ff2cf4f7df567306fdecd9ce Mon Sep 17 00:00:00 2001 From: NikiforovAll Date: Sat, 6 Jun 2020 06:39:36 +0300 Subject: [PATCH 2/4] remove typo --- .../DefaultBuilder/CommandLineExecutorService.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/System.CommandLine.Hosting/DefaultBuilder/CommandLineExecutorService.cs b/src/System.CommandLine.Hosting/DefaultBuilder/CommandLineExecutorService.cs index ac1315f13c..b22ecf6326 100644 --- a/src/System.CommandLine.Hosting/DefaultBuilder/CommandLineExecutorService.cs +++ b/src/System.CommandLine.Hosting/DefaultBuilder/CommandLineExecutorService.cs @@ -32,7 +32,6 @@ public CommandLineExecutorService( this.invocation = invocation; this.parseResult = parseResult; this.loggerFactory = loggerFactory; - // Logger = loggerFactory.CreateLogger("Microsoft.AspNetCore.Hosting.Diagnostics"); Configuration = configuration; } From 6d3391b85111eb2926c4ea1d71535489507da2ae Mon Sep 17 00:00:00 2001 From: NikiforovAll Date: Sun, 7 Jun 2020 22:01:03 +0300 Subject: [PATCH 3/4] Create CommandLineBuilder once --- .../HostingTests.cs | 24 ++++++++++++---- .../CommandLineExecutorService.cs | 19 ++++++------- .../DefaultBuilder/CommandLineHost.cs | 2 -- .../GenericCommandLineHostBuilder.cs | 1 - .../GenericHostBuilderExtensions.cs | 28 +++++++++++++++++-- 5 files changed, 52 insertions(+), 22 deletions(-) diff --git a/src/System.CommandLine.Hosting.Tests/HostingTests.cs b/src/System.CommandLine.Hosting.Tests/HostingTests.cs index 3d3779b01f..b6ff8e01d1 100644 --- a/src/System.CommandLine.Hosting.Tests/HostingTests.cs +++ b/src/System.CommandLine.Hosting.Tests/HostingTests.cs @@ -230,12 +230,12 @@ public static void UseHost_binds_parsed_arguments_to_options() } - [Fact] - public static async Task CommandLineHost_default() + [Fact(Skip ="WIP")] + public static async Task CommandLineHost_creates_host_for_simple_command() { //Arrange // var args = new string[] { $"--foo", "42" }; - // MyOptions options = null; + MyOptions options = null; IHost hostToBind = null; var rootCmd = new RootCommand(); @@ -256,8 +256,22 @@ await CommandLineHost.CreateDefaultBuilder() .Build() .RunAsync(tokenSource?.Token ?? default); // Assert - // Assert.NotNull(hostToBind); - // Assert.Equal(42, options.MyArgument); + Assert.NotNull(hostToBind); + Assert.Equal(42, options.MyArgument); + } + + [Fact] + public static async Task CommandLineHost_contains_errors_in_ParseResult_service_for_not_mapped_input() + { + //Arrange + // Act + var host = CommandLineHost.CreateDefaultBuilder(new string[]{"--foo", "bar"}) + .ConfigureCommandLineDefaults((CommandLineBuilder builder) => {}) + .Build(); + var parseResult = host.Services.GetService(); + await host.StartAsync(); + parseResult.Errors.Should().NotBeEmpty(); + //TODO: clarify how parsing errors and command execution exceptions should be handled } private class MyOptions diff --git a/src/System.CommandLine.Hosting/DefaultBuilder/CommandLineExecutorService.cs b/src/System.CommandLine.Hosting/DefaultBuilder/CommandLineExecutorService.cs index b22ecf6326..465dbeadfa 100644 --- a/src/System.CommandLine.Hosting/DefaultBuilder/CommandLineExecutorService.cs +++ b/src/System.CommandLine.Hosting/DefaultBuilder/CommandLineExecutorService.cs @@ -3,9 +3,9 @@ using System.CommandLine.Invocation; using System.CommandLine.Parsing; +using System.Linq; using System.Threading; using System.Threading.Tasks; -using Microsoft.Extensions.Configuration; using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; @@ -13,36 +13,33 @@ namespace System.CommandLine.Hosting { internal class CommandLineExecutorService : IHostedService { - private readonly ILoggerFactory loggerFactory; + private readonly ILogger logger; private readonly IHostApplicationLifetime appLifetime; private readonly IHost host; private readonly InvocationContext invocation; private readonly ParseResult parseResult; public CommandLineExecutorService( - ILoggerFactory loggerFactory, - IConfiguration configuration, + ILogger logger, IHostApplicationLifetime appLifetime, IHost host, InvocationContext invocation, ParseResult parseResult) { + this.logger = logger; this.appLifetime = appLifetime; this.host = host; this.invocation = invocation; this.parseResult = parseResult; - this.loggerFactory = loggerFactory; - Configuration = configuration; } - public ILogger Logger { get; } - // Only for high level lifetime events - public IConfiguration Configuration { get; } - public async Task StartAsync(CancellationToken cancellationToken) { - // TODO: consider how to make more robust invocation.BindingContext.AddService(typeof(IHost), _ => host); + if(parseResult.Errors.Any()) + { + logger.LogWarning($"Executing {nameof(Parser)} with errors: {parseResult.Errors.Count}"); + } await parseResult.InvokeAsync(); appLifetime.StopApplication(); } diff --git a/src/System.CommandLine.Hosting/DefaultBuilder/CommandLineHost.cs b/src/System.CommandLine.Hosting/DefaultBuilder/CommandLineHost.cs index 27a9d49ec1..5a8fafb8f9 100644 --- a/src/System.CommandLine.Hosting/DefaultBuilder/CommandLineHost.cs +++ b/src/System.CommandLine.Hosting/DefaultBuilder/CommandLineHost.cs @@ -31,8 +31,6 @@ public static IHostBuilder CreateDefaultBuilder(string[] args) return builder; // TODO: add remaining args parsing // var argsRemaining = invocation.ParseResult.UnparsedTokens.ToArray(); - // TODO: - // ConfigureCommandLineDefaults: remove redundant call GenericCommandLineHostBuilder.Configure } } } diff --git a/src/System.CommandLine.Hosting/DefaultBuilder/GenericCommandLineHostBuilder.cs b/src/System.CommandLine.Hosting/DefaultBuilder/GenericCommandLineHostBuilder.cs index bd2ec506b6..41d4ab5b85 100644 --- a/src/System.CommandLine.Hosting/DefaultBuilder/GenericCommandLineHostBuilder.cs +++ b/src/System.CommandLine.Hosting/DefaultBuilder/GenericCommandLineHostBuilder.cs @@ -31,7 +31,6 @@ public void Configure(Action configure) configure(commandLineBuilder); Parser parser = commandLineBuilder.Build(); ParseResult parseResult = parser.Parse(Args); - // TODO: throw/handle error in parse result at this stage var invocation = new InvocationContext(parseResult); AddSystemCommandLine(builder, invocation); builder.ConfigureHostConfiguration(config => diff --git a/src/System.CommandLine.Hosting/DefaultBuilder/GenericHostBuilderExtensions.cs b/src/System.CommandLine.Hosting/DefaultBuilder/GenericHostBuilderExtensions.cs index 6387d8c8e9..356e30e420 100644 --- a/src/System.CommandLine.Hosting/DefaultBuilder/GenericHostBuilderExtensions.cs +++ b/src/System.CommandLine.Hosting/DefaultBuilder/GenericHostBuilderExtensions.cs @@ -23,11 +23,33 @@ public static IHostBuilder ConfigureCommandLineDefaults( Action configure, string[] args = default) { - var host = new GenericCommandLineHostBuilder(builder, args); - host.Configure(configure); - builder.ConfigureServices((context, services) => + AddGenericCommandLineHostBuilder(builder, args, out var cmdHostBuilder); + cmdHostBuilder.Configure(configure); + builder.ConfigureServices((context, services) => services.AddHostedService()); return builder; } + + /// + /// Adds to . + /// + /// + /// + /// + /// when new builder is intialized + private static bool AddGenericCommandLineHostBuilder(IHostBuilder builder, string[] args, out GenericCommandLineHostBuilder cmdHostBuilder) + { + var hostBuilderState = builder.Properties; + var cacheKey = nameof(GenericCommandLineHostBuilder); + hostBuilderState.TryGetValue(cacheKey, out var initializedHost); + cmdHostBuilder = initializedHost as GenericCommandLineHostBuilder; + if (cmdHostBuilder is object) + { + return false; + } + cmdHostBuilder = new GenericCommandLineHostBuilder(builder, args); + hostBuilderState[cacheKey] = cmdHostBuilder; + return true; + } } } From 4355046940707ffac4c1f151f92c02d3c7ed0a8a Mon Sep 17 00:00:00 2001 From: NikiforovAll Date: Sun, 7 Jun 2020 22:04:53 +0300 Subject: [PATCH 4/4] Add copyright header --- .../DefaultBuilder/CommandLineHost.cs | 3 +++ .../DefaultBuilder/GenericHostBuilderExtensions.cs | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/System.CommandLine.Hosting/DefaultBuilder/CommandLineHost.cs b/src/System.CommandLine.Hosting/DefaultBuilder/CommandLineHost.cs index 5a8fafb8f9..0523c8f266 100644 --- a/src/System.CommandLine.Hosting/DefaultBuilder/CommandLineHost.cs +++ b/src/System.CommandLine.Hosting/DefaultBuilder/CommandLineHost.cs @@ -1,3 +1,6 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + using Microsoft.Extensions.Hosting; using System.CommandLine.Builder; diff --git a/src/System.CommandLine.Hosting/DefaultBuilder/GenericHostBuilderExtensions.cs b/src/System.CommandLine.Hosting/DefaultBuilder/GenericHostBuilderExtensions.cs index 356e30e420..bf49750b5a 100644 --- a/src/System.CommandLine.Hosting/DefaultBuilder/GenericHostBuilderExtensions.cs +++ b/src/System.CommandLine.Hosting/DefaultBuilder/GenericHostBuilderExtensions.cs @@ -1,6 +1,8 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + using System.CommandLine.Builder; using Microsoft.Extensions.DependencyInjection; -// using System.CommandLine.Hosting; using Microsoft.Extensions.Hosting; namespace System.CommandLine.Hosting