From d556325933c84ee4926d3863a5ddb6a4f6e4b09d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B8is=C3=A6ther=20Rasch?= Date: Tue, 20 Jul 2021 14:38:07 +0200 Subject: [PATCH 1/6] Simplified usage of BindingContext resolved command handlers --- .../Invocation/CommandHandlerTests.cs | 35 ++++++++++++++++++- .../Binding/BindingContext.cs | 24 ++++++++++--- .../Builder/CommandLineBuilderExtensions.cs | 12 +++++++ .../Invocation/CommandHandler.cs | 4 +++ 4 files changed, 70 insertions(+), 5 deletions(-) diff --git a/src/System.CommandLine.Tests/Invocation/CommandHandlerTests.cs b/src/System.CommandLine.Tests/Invocation/CommandHandlerTests.cs index f799649ddc..d7e31dc6d5 100644 --- a/src/System.CommandLine.Tests/Invocation/CommandHandlerTests.cs +++ b/src/System.CommandLine.Tests/Invocation/CommandHandlerTests.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System.CommandLine.Binding; +using System.CommandLine.Builder; using System.CommandLine.Invocation; using System.CommandLine.IO; using System.CommandLine.Parsing; @@ -322,7 +323,6 @@ public async Task Method_parameters_of_type_InvocationContext_receive_the_curren boundContext.ParseResult.ValueForOption(option).Should().Be(123); } - private class ExecuteTestClass { public string boundName = default; @@ -507,5 +507,38 @@ public class OverridenVirtualTestCommandHandler : VirtualTestCommandHandler public override Task InvokeAsync(InvocationContext context) => Task.FromResult(41); } + + [Fact] + public static void FromBindingContext_forwards_invocation_to_bound_handler_type() + { + var command = new RootCommand + { + Handler = CommandHandler.FromBindingContext() + }; + command.Handler.Should().NotBeOfType(); + var parser = new CommandLineBuilder(command) + .ConfigureBindingContext(context => context.AddService()) + .Build(); + + var console = new TestConsole(); + parser.Invoke(Array.Empty(), console); + console.Out.ToString().Should().Be(typeof(BindingContextResolvedCommandHandler).FullName); + } + + public class BindingContextResolvedCommandHandler : ICommandHandler + { + public BindingContextResolvedCommandHandler(IConsole console) + { + Console = console; + } + + public IConsole Console { get; } + + public Task InvokeAsync(InvocationContext context) + { + Console.Out.Write(GetType().FullName); + return Task.FromResult(0); + } + } } } diff --git a/src/System.CommandLine/Binding/BindingContext.cs b/src/System.CommandLine/Binding/BindingContext.cs index a80a417287..789d2d9074 100644 --- a/src/System.CommandLine/Binding/BindingContext.cs +++ b/src/System.CommandLine/Binding/BindingContext.cs @@ -51,7 +51,7 @@ public IConsole Console internal ServiceProvider ServiceProvider { get; } - public void AddModelBinder(ModelBinder binder) => + public void AddModelBinder(ModelBinder binder) => _modelBindersByValueDescriptor.Add(binder.ValueDescriptor.ValueType, binder); public ModelBinder GetModelBinder(IValueDescriptor valueDescriptor) @@ -67,7 +67,7 @@ public void AddService(Type serviceType, Func factory) { ServiceProvider.AddService(serviceType, factory); } - + public void AddService(Func factory) { if (factory is null) @@ -78,6 +78,22 @@ public void AddService(Func factory) ServiceProvider.AddService(typeof(T), s => factory(s)); } + public void AddService(Type serviceType) + { + object factory(IServiceProvider serviceProvider) + { + var bindingContext = + serviceProvider.GetService(typeof(BindingContext)) as BindingContext + ?? this; + var valueDescriptor = new ModelBinder.AnonymousValueDescriptor(serviceType); + var modelBinder = bindingContext.GetModelBinder(valueDescriptor); + return modelBinder.CreateInstance(bindingContext)!; + } + AddService(serviceType, factory); + } + + public void AddService() => AddService(typeof(T)); + internal bool TryGetValueSource( IValueDescriptor valueDescriptor, [MaybeNullWhen(false)] out IValueSource valueSource) @@ -108,8 +124,8 @@ public void AddService(Func factory) else { var parsed = ArgumentConverter.ConvertObject( - valueDescriptor as IArgument ?? new Argument(valueDescriptor.ValueName), - valueDescriptor.ValueType, + valueDescriptor as IArgument ?? new Argument(valueDescriptor.ValueName), + valueDescriptor.ValueType, value, resources); diff --git a/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs b/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs index ee966c9611..c4b2393d7c 100644 --- a/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs +++ b/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs @@ -143,6 +143,18 @@ public static CommandLineBuilder CancelOnProcessTermination(this CommandLineBuil return builder; } + public static CommandLineBuilder ConfigureBindingContext( + this CommandLineBuilder builder, + Action configureBindingContext) + { + builder.AddMiddleware((context, next) => + { + configureBindingContext?.Invoke(context.BindingContext); + return next(context); + }, default(MiddlewareOrder)); + return builder; + } + public static CommandLineBuilder EnableDirectives( this CommandLineBuilder builder, bool value = true) diff --git a/src/System.CommandLine/Invocation/CommandHandler.cs b/src/System.CommandLine/Invocation/CommandHandler.cs index 6b6464852e..ab1e16b910 100644 --- a/src/System.CommandLine/Invocation/CommandHandler.cs +++ b/src/System.CommandLine/Invocation/CommandHandler.cs @@ -282,6 +282,10 @@ public static class CommandHandler Func> action) => HandlerDescriptor.FromDelegate(action).GetCommandHandler(); + public static ICommandHandler FromBindingContext() + where THandler : ICommandHandler => + Create((InvocationContext context, THandler handler) => handler.InvokeAsync(context)); + internal static async Task GetExitCodeAsync(object value, InvocationContext context) { switch (value) From 2f2796b13d15b3965bfd6e4a71530cb280ec6a05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B8is=C3=A6ther=20Rasch?= Date: Wed, 21 Jul 2021 12:51:23 +0200 Subject: [PATCH 2/6] Add AddService() to BindingContext --- src/System.CommandLine/Binding/BindingContext.cs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/System.CommandLine/Binding/BindingContext.cs b/src/System.CommandLine/Binding/BindingContext.cs index 789d2d9074..c11c90ca45 100644 --- a/src/System.CommandLine/Binding/BindingContext.cs +++ b/src/System.CommandLine/Binding/BindingContext.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System.Collections.Generic; @@ -78,21 +78,24 @@ public void AddService(Func factory) ServiceProvider.AddService(typeof(T), s => factory(s)); } - public void AddService(Type serviceType) + public void AddService(Type serviceType, Type? implementationType = null) { object factory(IServiceProvider serviceProvider) { var bindingContext = serviceProvider.GetService(typeof(BindingContext)) as BindingContext ?? this; - var valueDescriptor = new ModelBinder.AnonymousValueDescriptor(serviceType); + var valueDescriptor = new ModelBinder.AnonymousValueDescriptor(implementationType); var modelBinder = bindingContext.GetModelBinder(valueDescriptor); return modelBinder.CreateInstance(bindingContext)!; } AddService(serviceType, factory); } - public void AddService() => AddService(typeof(T)); + public void AddService() => + AddService(typeof(TService), typeof(TImplementation)); + + public void AddService() => AddService(); internal bool TryGetValueSource( IValueDescriptor valueDescriptor, From b811921ca741bc9d9a66e034e4f536c2a6ec567e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B8is=C3=A6ther=20Rasch?= Date: Wed, 21 Jul 2021 12:51:50 +0200 Subject: [PATCH 3/6] Argument validation for AddService methods --- src/System.CommandLine/Binding/BindingContext.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/System.CommandLine/Binding/BindingContext.cs b/src/System.CommandLine/Binding/BindingContext.cs index c11c90ca45..1bd2b31685 100644 --- a/src/System.CommandLine/Binding/BindingContext.cs +++ b/src/System.CommandLine/Binding/BindingContext.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System.Collections.Generic; @@ -65,21 +65,21 @@ public ModelBinder GetModelBinder(IValueDescriptor valueDescriptor) public void AddService(Type serviceType, Func factory) { + _ = serviceType ?? throw new ArgumentNullException(nameof(serviceType)); + _ = factory ?? throw new ArgumentNullException(nameof(factory)); ServiceProvider.AddService(serviceType, factory); } public void AddService(Func factory) { - if (factory is null) - { - throw new ArgumentNullException(nameof(factory)); - } - + _ = factory ?? throw new ArgumentNullException(nameof(factory)); ServiceProvider.AddService(typeof(T), s => factory(s)); } public void AddService(Type serviceType, Type? implementationType = null) { + _ = serviceType ?? throw new ArgumentNullException(nameof(serviceType)); + implementationType ??= serviceType; object factory(IServiceProvider serviceProvider) { var bindingContext = From b050e7ac0a4214bfe29fc69773224e3f0ef4bf1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B8is=C3=A6ther=20Rasch?= Date: Thu, 22 Jul 2021 08:41:22 +0200 Subject: [PATCH 4/6] Add additional test case for ConfigureBindingContext --- .../Invocation/CommandHandlerTests.cs | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/src/System.CommandLine.Tests/Invocation/CommandHandlerTests.cs b/src/System.CommandLine.Tests/Invocation/CommandHandlerTests.cs index d7e31dc6d5..7d60a4b4c0 100644 --- a/src/System.CommandLine.Tests/Invocation/CommandHandlerTests.cs +++ b/src/System.CommandLine.Tests/Invocation/CommandHandlerTests.cs @@ -525,6 +525,24 @@ public static void FromBindingContext_forwards_invocation_to_bound_handler_type( console.Out.ToString().Should().Be(typeof(BindingContextResolvedCommandHandler).FullName); } + [Fact] + public static void Subsequent_call_to_configure_overrides_service_registration() + { + BindingContextCommandHandlerAction action = (handler, Console) => + { + handler.Should().BeOfType(); + }; + var parser = new CommandLineBuilder(new RootCommand + { + Handler = CommandHandler.FromBindingContext() + }) + .ConfigureBindingContext(context => context.AddService(_ => action)) + .ConfigureBindingContext(context => context.AddService()) + .ConfigureBindingContext(context => context.AddService()) + .Build(); + parser.Invoke(Array.Empty(), new TestConsole()); + } + public class BindingContextResolvedCommandHandler : ICommandHandler { public BindingContextResolvedCommandHandler(IConsole console) @@ -540,5 +558,50 @@ public Task InvokeAsync(InvocationContext context) return Task.FromResult(0); } } + + public interface IBindingContextCommandHandlerInterface : ICommandHandler + { + } + + public class BindingContextCommandHandler1 : IBindingContextCommandHandlerInterface + { + private readonly BindingContextCommandHandlerAction invokeAction; + + public BindingContextCommandHandler1(IConsole console, + BindingContextCommandHandlerAction invokeAction) + { + Console = console; + this.invokeAction = invokeAction; + } + + public IConsole Console { get; } + public Task InvokeAsync(InvocationContext context) + { + invokeAction(this, Console); + return Task.FromResult(0); + } + } + + public class BindingContextCommandHandler2 : IBindingContextCommandHandlerInterface + { + private readonly BindingContextCommandHandlerAction invokeAction; + + public BindingContextCommandHandler2(IConsole console, + BindingContextCommandHandlerAction invokeAction) + { + Console = console; + this.invokeAction = invokeAction; + } + + public IConsole Console { get; } + + public Task InvokeAsync(InvocationContext context) + { + invokeAction(this, Console); + return Task.FromResult(0); + } + } + + public delegate void BindingContextCommandHandlerAction(ICommandHandler handler, IConsole console); } } From 5db4df8aa57b46a17b3a60c57c0a92a847fad59b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B8is=C3=A6ther=20Rasch?= Date: Sun, 8 Aug 2021 20:34:51 +0200 Subject: [PATCH 5/6] Create own test for returned wrapper instance when calling CommandHandler.FromBindingContext resolves https://github.com/dotnet/command-line-api/pull/1358/files/b050e7ac0a4214bfe29fc69773224e3f0ef4bf1a#r676031584 --- .../Invocation/CommandHandlerTests.cs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/System.CommandLine.Tests/Invocation/CommandHandlerTests.cs b/src/System.CommandLine.Tests/Invocation/CommandHandlerTests.cs index 7d60a4b4c0..04ee2bc448 100644 --- a/src/System.CommandLine.Tests/Invocation/CommandHandlerTests.cs +++ b/src/System.CommandLine.Tests/Invocation/CommandHandlerTests.cs @@ -515,7 +515,6 @@ public static void FromBindingContext_forwards_invocation_to_bound_handler_type( { Handler = CommandHandler.FromBindingContext() }; - command.Handler.Should().NotBeOfType(); var parser = new CommandLineBuilder(command) .ConfigureBindingContext(context => context.AddService()) .Build(); @@ -525,6 +524,13 @@ public static void FromBindingContext_forwards_invocation_to_bound_handler_type( console.Out.ToString().Should().Be(typeof(BindingContextResolvedCommandHandler).FullName); } + [Fact] + public static void FromBindingContext_returns_a_wrapper_type_instance() + { + ICommandHandler handler = CommandHandler.FromBindingContext(); + handler.Should().NotBeOfType(); + } + [Fact] public static void Subsequent_call_to_configure_overrides_service_registration() { From 765d3a57f17a5eb4b2235e026d05a3ad3ce12609 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B8is=C3=A6ther=20Rasch?= Date: Sun, 8 Aug 2021 20:37:50 +0200 Subject: [PATCH 6/6] resolve https://github.com/dotnet/command-line-api/pull/1358/files/b050e7ac0a4214bfe29fc69773224e3f0ef4bf1a#r676031042 --- .../Invocation/CommandHandlerTests.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/System.CommandLine.Tests/Invocation/CommandHandlerTests.cs b/src/System.CommandLine.Tests/Invocation/CommandHandlerTests.cs index 04ee2bc448..d6289d5c5d 100644 --- a/src/System.CommandLine.Tests/Invocation/CommandHandlerTests.cs +++ b/src/System.CommandLine.Tests/Invocation/CommandHandlerTests.cs @@ -534,9 +534,10 @@ public static void FromBindingContext_returns_a_wrapper_type_instance() [Fact] public static void Subsequent_call_to_configure_overrides_service_registration() { + ICommandHandler invokedHandler = null; BindingContextCommandHandlerAction action = (handler, Console) => { - handler.Should().BeOfType(); + invokedHandler = handler; }; var parser = new CommandLineBuilder(new RootCommand { @@ -547,6 +548,9 @@ public static void Subsequent_call_to_configure_overrides_service_registration() .ConfigureBindingContext(context => context.AddService()) .Build(); parser.Invoke(Array.Empty(), new TestConsole()); + + invokedHandler.Should().NotBeNull(); + invokedHandler.Should().BeOfType(); } public class BindingContextResolvedCommandHandler : ICommandHandler