From b8c60717d5a2c6e25343a704f055b4ff1228b7bc Mon Sep 17 00:00:00 2001 From: Oleksandr Shustov Date: Fri, 6 Nov 2020 20:37:46 +0200 Subject: [PATCH 1/4] add a base type for custom validators --- CliFx/ArgumentValueValidator.cs | 18 ++++++++++ CliFx/Attributes/CommandOptionAttribute.cs | 5 +++ CliFx/Attributes/CommandParameterAttribute.cs | 5 +++ CliFx/Domain/CommandArgumentSchema.cs | 36 +++++++++++++++++-- CliFx/Domain/CommandOptionSchema.cs | 9 +++-- CliFx/Domain/CommandParameterSchema.cs | 14 ++++++-- CliFx/Exceptions/CliFxException.cs | 8 +++++ CliFx/IArgumentValueValidator.cs | 7 ++++ .../Extensions/CollectionExtensions.cs | 7 ++++ CliFx/ValidationResult.cs | 30 ++++++++++++++++ 10 files changed, 130 insertions(+), 9 deletions(-) create mode 100644 CliFx/ArgumentValueValidator.cs create mode 100644 CliFx/IArgumentValueValidator.cs create mode 100644 CliFx/ValidationResult.cs diff --git a/CliFx/ArgumentValueValidator.cs b/CliFx/ArgumentValueValidator.cs new file mode 100644 index 0000000..69797bf --- /dev/null +++ b/CliFx/ArgumentValueValidator.cs @@ -0,0 +1,18 @@ +namespace CliFx +{ + /// + /// A base type for custom validators. + /// + public abstract class ArgumentValueValidator : IArgumentValueValidator + { + /// + /// Your validation logic have to be implemented in this method. + /// + public abstract ValidationResult Validate(T value); + + /// + /// Non-generic method, will be called by the framework. + /// + public ValidationResult Validate(object value) => Validate((T) value); + } +} diff --git a/CliFx/Attributes/CommandOptionAttribute.cs b/CliFx/Attributes/CommandOptionAttribute.cs index 787ba53..66e9d61 100644 --- a/CliFx/Attributes/CommandOptionAttribute.cs +++ b/CliFx/Attributes/CommandOptionAttribute.cs @@ -42,6 +42,11 @@ namespace CliFx.Attributes /// public Type? Converter { get; set; } + /// + /// Type of a converter to use for the option value evaluating. + /// + public Type[]? Validators { get; set; } + /// /// Initializes an instance of . /// diff --git a/CliFx/Attributes/CommandParameterAttribute.cs b/CliFx/Attributes/CommandParameterAttribute.cs index 67f5c87..9e9bd7e 100644 --- a/CliFx/Attributes/CommandParameterAttribute.cs +++ b/CliFx/Attributes/CommandParameterAttribute.cs @@ -31,6 +31,11 @@ namespace CliFx.Attributes /// public Type? Converter { get; set; } + /// + /// Type of a converter to use for the option value evaluating. + /// + public Type[]? Validators { get; set; } + /// /// Initializes an instance of . /// diff --git a/CliFx/Domain/CommandArgumentSchema.cs b/CliFx/Domain/CommandArgumentSchema.cs index dc726fb..06dd49f 100644 --- a/CliFx/Domain/CommandArgumentSchema.cs +++ b/CliFx/Domain/CommandArgumentSchema.cs @@ -19,11 +19,15 @@ namespace CliFx.Domain protected Type? Converter { get; set; } - protected CommandArgumentSchema(PropertyInfo? property, string? description, Type? converter = null) + private readonly Type[]? _validators; + + protected CommandArgumentSchema(PropertyInfo? property, string? description, Type? converter = null, Type[]? validators = null) { Property = property; Description = description; Converter = converter; + + _validators = validators; } private Type? TryGetEnumerableArgumentUnderlyingType() => @@ -120,8 +124,15 @@ namespace CliFx.Domain } } - public void BindOn(ICommand command, IReadOnlyList values) => - Property?.SetValue(command, Convert(values)); + public void BindOn(ICommand command, IReadOnlyList values) + { + var value = Convert(values); + + if (_validators.NotEmpty()) + Validate(value); + + Property?.SetValue(command, value); + } public void BindOn(ICommand command, params string[] values) => BindOn(command, (IReadOnlyList) values); @@ -141,6 +152,25 @@ namespace CliFx.Domain return Array.Empty(); } + + private void Validate(object? value) + { + if (value is null) + return; + + var failed = new List(); + foreach (var validator in _validators!) + { + var result = validator.InstanceOf().Validate(value!); + if (result.IsValid) + continue; + + failed.Add(result); + } + + if (failed.NotEmpty()) + throw CliFxException.ValueValidationFailed(this, failed.Select(x => x.ErrorMessage!)); + } } internal partial class CommandArgumentSchema diff --git a/CliFx/Domain/CommandOptionSchema.cs b/CliFx/Domain/CommandOptionSchema.cs index f5476c3..960b0e7 100644 --- a/CliFx/Domain/CommandOptionSchema.cs +++ b/CliFx/Domain/CommandOptionSchema.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Reflection; @@ -24,8 +25,9 @@ namespace CliFx.Domain string? environmentVariableName, bool isRequired, string? description, - Type? converter = null) - : base(property, description, converter) + Type? converter = null, + Type[]? validators = null) + : base(property, description, converter, validators) { Name = name; ShortName = shortName; @@ -99,7 +101,8 @@ namespace CliFx.Domain attribute.EnvironmentVariableName, attribute.IsRequired, attribute.Description, - attribute.Converter + attribute.Converter, + attribute.Validators ); } } diff --git a/CliFx/Domain/CommandParameterSchema.cs b/CliFx/Domain/CommandParameterSchema.cs index ea70137..7b6cbb5 100644 --- a/CliFx/Domain/CommandParameterSchema.cs +++ b/CliFx/Domain/CommandParameterSchema.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; using System.Reflection; using System.Text; @@ -12,8 +13,14 @@ namespace CliFx.Domain public string Name { get; } - public CommandParameterSchema(PropertyInfo? property, int order, string name, string? description, Type? converter = null) - : base(property, description, converter) + public CommandParameterSchema( + PropertyInfo? property, + int order, + string name, + string? description, + Type? converter = null, + Type[]? validators = null) + : base(property, description, converter, validators) { Order = order; Name = name; @@ -52,7 +59,8 @@ namespace CliFx.Domain attribute.Order, name, attribute.Description, - attribute.Converter + attribute.Converter, + attribute.Validators ); } } diff --git a/CliFx/Exceptions/CliFxException.cs b/CliFx/Exceptions/CliFxException.cs index f02c7f8..dfef45d 100644 --- a/CliFx/Exceptions/CliFxException.cs +++ b/CliFx/Exceptions/CliFxException.cs @@ -389,5 +389,13 @@ Unrecognized options provided: return new CliFxException(message.Trim()); } + + internal static CliFxException ValueValidationFailed(CommandArgumentSchema argument, IEnumerable errors) + { + var message = $@" +The validation of the provided value for {argument.Property!.Name} is failed because: {errors.JoinToString(Environment.NewLine)}"; + + return new CliFxException(message.Trim()); + } } } \ No newline at end of file diff --git a/CliFx/IArgumentValueValidator.cs b/CliFx/IArgumentValueValidator.cs new file mode 100644 index 0000000..8c45b37 --- /dev/null +++ b/CliFx/IArgumentValueValidator.cs @@ -0,0 +1,7 @@ +namespace CliFx +{ + internal interface IArgumentValueValidator + { + ValidationResult Validate(object value); + } +} diff --git a/CliFx/Internal/Extensions/CollectionExtensions.cs b/CliFx/Internal/Extensions/CollectionExtensions.cs index 9ef38fd..38e29ec 100644 --- a/CliFx/Internal/Extensions/CollectionExtensions.cs +++ b/CliFx/Internal/Extensions/CollectionExtensions.cs @@ -1,4 +1,5 @@ using System.Collections.Generic; +using System.Linq; namespace CliFx.Internal.Extensions { @@ -9,5 +10,11 @@ namespace CliFx.Internal.Extensions foreach (var item in items) source.Remove(item); } + + public static bool IsNullOrEmpty(this IEnumerable? source) => + !source?.Any() ?? true; + + public static bool NotEmpty(this IEnumerable? source) => + !source.IsNullOrEmpty(); } } \ No newline at end of file diff --git a/CliFx/ValidationResult.cs b/CliFx/ValidationResult.cs new file mode 100644 index 0000000..4a04f4e --- /dev/null +++ b/CliFx/ValidationResult.cs @@ -0,0 +1,30 @@ +namespace CliFx +{ + /// + /// A tiny object that represents a result of the validation. + /// + public class ValidationResult + { + /// + /// False if there is no error message, otherwise - true. + /// + public bool IsValid => ErrorMessage == null; + + /// + /// Contains an information about the reasons of failed validation. + /// + public string? ErrorMessage { get; private set; } + + private ValidationResult() { } + + /// + /// Creates Ok result, means that the validation is passed. + /// + public static ValidationResult Ok() => new ValidationResult() { }; + + /// + /// Creates Error result, means that the validation failed. + /// + public static ValidationResult Error(string message) => new ValidationResult() { ErrorMessage = message }; + } +} From 144d3592fb79ef2156c9252f33bc23861c9c6139 Mon Sep 17 00:00:00 2001 From: Oleksandr Shustov Date: Sat, 7 Nov 2020 21:34:12 +0200 Subject: [PATCH 2/4] cleanup after merge --- CliFx/Domain/CommandArgumentSchema.cs | 10 +++++----- CliFx/Domain/CommandOptionSchema.cs | 9 ++++----- CliFx/Domain/CommandParameterSchema.cs | 5 ++--- CliFx/Domain/RootSchema.cs | 24 ++++++++++++++++++++++++ CliFx/Exceptions/CliFxException.cs | 26 ++++++++++++++++++++++++++ 5 files changed, 61 insertions(+), 13 deletions(-) diff --git a/CliFx/Domain/CommandArgumentSchema.cs b/CliFx/Domain/CommandArgumentSchema.cs index 0aa4fbf..0cd0cff 100644 --- a/CliFx/Domain/CommandArgumentSchema.cs +++ b/CliFx/Domain/CommandArgumentSchema.cs @@ -19,7 +19,7 @@ namespace CliFx.Domain public Type? ConverterType { get; } - private readonly Type[]? _validators; + public readonly Type[]? ValidatorTypes; protected CommandArgumentSchema(PropertyInfo? property, string? description, Type? converterType = null, Type[]? validators = null) { @@ -27,7 +27,7 @@ namespace CliFx.Domain Description = description; ConverterType = converterType; - _validators = validators; + ValidatorTypes = validators; } private Type? TryGetEnumerableArgumentUnderlyingType() => @@ -140,7 +140,7 @@ namespace CliFx.Domain { var value = Convert(values); - if (_validators.NotEmpty()) + if (ValidatorTypes.NotEmpty()) Validate(value); Property?.SetValue(command, value); @@ -171,9 +171,9 @@ namespace CliFx.Domain return; var failed = new List(); - foreach (var validator in _validators!) + foreach (var validator in ValidatorTypes!) { - var result = validator.InstanceOf().Validate(value!); + var result = validator.CreateInstance().Validate(value!); if (result.IsValid) continue; diff --git a/CliFx/Domain/CommandOptionSchema.cs b/CliFx/Domain/CommandOptionSchema.cs index dc342f4..941a23f 100644 --- a/CliFx/Domain/CommandOptionSchema.cs +++ b/CliFx/Domain/CommandOptionSchema.cs @@ -1,5 +1,4 @@ using System; -using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Reflection; @@ -26,8 +25,8 @@ namespace CliFx.Domain bool isRequired, string? description, Type? converterType = null, - Type[]? validators = null) - : base(property, description, converterType, validators) + Type[]? validatorTypes = null) + : base(property, description, converterType, validatorTypes) { Name = name; ShortName = shortName; @@ -110,9 +109,9 @@ namespace CliFx.Domain internal partial class CommandOptionSchema { public static CommandOptionSchema HelpOption { get; } = - new CommandOptionSchema(null, "help", 'h', null, false, "Shows help text.", null); + new CommandOptionSchema(null, "help", 'h', null, false, "Shows help text.", converterType: null, validatorTypes: null); public static CommandOptionSchema VersionOption { get; } = - new CommandOptionSchema(null, "version", null, null, false, "Shows version information.", null); + new CommandOptionSchema(null, "version", null, null, false, "Shows version information.", converterType: null, validatorTypes: null); } } \ No newline at end of file diff --git a/CliFx/Domain/CommandParameterSchema.cs b/CliFx/Domain/CommandParameterSchema.cs index 7aa556e..814c8d6 100644 --- a/CliFx/Domain/CommandParameterSchema.cs +++ b/CliFx/Domain/CommandParameterSchema.cs @@ -1,5 +1,4 @@ using System; -using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; using System.Reflection; using System.Text; @@ -19,8 +18,8 @@ namespace CliFx.Domain string name, string? description, Type? converterType = null, - Type[]? validators = null) - : base(property, description, converterType, validators) + Type[]? validatorTypes = null) + : base(property, description, converterType, validatorTypes) { Order = order; Name = name; diff --git a/CliFx/Domain/RootSchema.cs b/CliFx/Domain/RootSchema.cs index 431bc43..98fff68 100644 --- a/CliFx/Domain/RootSchema.cs +++ b/CliFx/Domain/RootSchema.cs @@ -126,6 +126,18 @@ namespace CliFx.Domain invalidConverterParameters ); } + + var invalidValidatorParameters = command.Parameters + .Where(p => p.ValidatorTypes != null && !p.ValidatorTypes.All(x => x.Implements(typeof(IArgumentValueValidator)))) + .ToArray(); + + if (invalidValidatorParameters.Any()) + { + throw CliFxException.ParametersWithInvalidValidators( + command, + invalidValidatorParameters + ); + } } private static void ValidateOptions(CommandSchema command) @@ -208,6 +220,18 @@ namespace CliFx.Domain invalidConverterOptions ); } + + var invalidValidatorOptions = command.Options + .Where(o => o.ValidatorTypes != null && !o.ValidatorTypes.All(x => x.Implements(typeof(IArgumentValueValidator)))) + .ToArray(); + + if (invalidValidatorOptions.Any()) + { + throw CliFxException.OptionsWithInvalidValidators( + command, + invalidValidatorOptions + ); + } } private static void ValidateCommands(IReadOnlyList commands) diff --git a/CliFx/Exceptions/CliFxException.cs b/CliFx/Exceptions/CliFxException.cs index cb3075f..8535621 100644 --- a/CliFx/Exceptions/CliFxException.cs +++ b/CliFx/Exceptions/CliFxException.cs @@ -185,6 +185,19 @@ Specified converter must implement {typeof(IArgumentValueConverter).FullName}."; return new CliFxException(message.Trim()); } + internal static CliFxException ParametersWithInvalidValidators( + CommandSchema command, + IReadOnlyList invalidParameters) + { + var message = $@" +Command '{command.Type.FullName}' is invalid because it contains {invalidParameters.Count} parameter(s) with invalid value validators: +{invalidParameters.JoinToString(Environment.NewLine)} + +Specified validator(s) must inherit from {typeof(ArgumentValueValidator<>).FullName}."; + + return new CliFxException(message.Trim()); + } + internal static CliFxException OptionsWithNoName( CommandSchema command, IReadOnlyList invalidOptions) @@ -269,6 +282,19 @@ Specified converter must implement {typeof(IArgumentValueConverter).FullName}."; return new CliFxException(message.Trim()); } + + internal static CliFxException OptionsWithInvalidValidators( + CommandSchema command, + IReadOnlyList invalidOptions) + { + var message = $@" +Command '{command.Type.FullName}' is invalid because it contains {invalidOptions.Count} option(s) with invalid validators: +{invalidOptions.JoinToString(Environment.NewLine)} + +Specified validators must inherit from {typeof(IArgumentValueValidator).FullName}."; + + return new CliFxException(message.Trim()); + } } // End-user-facing exceptions From 4e12aefafbdea9cdf9ff59cc8f77e665dd85a459 Mon Sep 17 00:00:00 2001 From: Oleksandr Shustov Date: Sat, 7 Nov 2020 21:46:32 +0200 Subject: [PATCH 3/4] add tests --- CliFx.Tests/ApplicationSpecs.cs | 40 +++++++++++++++++++ .../InvalidCustomValidatorOptionCommand.cs | 16 ++++++++ .../InvalidCustomValidatorParameterCommand.cs | 16 ++++++++ 3 files changed, 72 insertions(+) create mode 100644 CliFx.Tests/Commands/Invalid/InvalidCustomValidatorOptionCommand.cs create mode 100644 CliFx.Tests/Commands/Invalid/InvalidCustomValidatorParameterCommand.cs diff --git a/CliFx.Tests/ApplicationSpecs.cs b/CliFx.Tests/ApplicationSpecs.cs index 8a2a7c7..135a2cc 100644 --- a/CliFx.Tests/ApplicationSpecs.cs +++ b/CliFx.Tests/ApplicationSpecs.cs @@ -252,6 +252,26 @@ namespace CliFx.Tests _output.WriteLine(stdErr.GetString()); } + [Fact] + public async Task Command_parameter_custom_validator_must_implement_the_corresponding_interface() + { + var (console, _, stdErr) = VirtualConsole.CreateBuffered(); + + var application = new CliApplicationBuilder() + .AddCommand() + .UseConsole(console) + .Build(); + + // Act + var exitCode = await application.RunAsync(Array.Empty()); + + // Assert + exitCode.Should().NotBe(0); + stdErr.GetString().Should().NotBeNullOrWhiteSpace(); + + _output.WriteLine(stdErr.GetString()); + } + [Fact] public async Task Command_options_must_have_names_that_are_not_empty() { @@ -411,5 +431,25 @@ namespace CliFx.Tests _output.WriteLine(stdErr.GetString()); } + + [Fact] + public async Task Command_option_custom_validator_must_implement_the_corresponding_interface() + { + var (console, _, stdErr) = VirtualConsole.CreateBuffered(); + + var application = new CliApplicationBuilder() + .AddCommand() + .UseConsole(console) + .Build(); + + // Act + var exitCode = await application.RunAsync(Array.Empty()); + + // Assert + exitCode.Should().NotBe(0); + stdErr.GetString().Should().NotBeNullOrWhiteSpace(); + + _output.WriteLine(stdErr.GetString()); + } } } \ No newline at end of file diff --git a/CliFx.Tests/Commands/Invalid/InvalidCustomValidatorOptionCommand.cs b/CliFx.Tests/Commands/Invalid/InvalidCustomValidatorOptionCommand.cs new file mode 100644 index 0000000..12f007e --- /dev/null +++ b/CliFx.Tests/Commands/Invalid/InvalidCustomValidatorOptionCommand.cs @@ -0,0 +1,16 @@ +using CliFx.Attributes; + +namespace CliFx.Tests.Commands.Invalid +{ + [Command] + public class InvalidCustomValidatorOptionCommand : SelfSerializeCommandBase + { + [CommandOption('f', Validators = new[] { typeof(Validator) })] + public string? Option { get; set; } + + public class Validator + { + public ValidationResult Validate(string value) => ValidationResult.Ok(); + } + } +} \ No newline at end of file diff --git a/CliFx.Tests/Commands/Invalid/InvalidCustomValidatorParameterCommand.cs b/CliFx.Tests/Commands/Invalid/InvalidCustomValidatorParameterCommand.cs new file mode 100644 index 0000000..77a5f1d --- /dev/null +++ b/CliFx.Tests/Commands/Invalid/InvalidCustomValidatorParameterCommand.cs @@ -0,0 +1,16 @@ +using CliFx.Attributes; + +namespace CliFx.Tests.Commands.Invalid +{ + [Command] + public class InvalidCustomValidatorParameterCommand : SelfSerializeCommandBase + { + [CommandParameter(0, Validators = new[] { typeof(Validator) })] + public string? Param { get; set; } + + public class Validator + { + public ValidationResult Validate(string value) => ValidationResult.Ok(); + } + } +} \ No newline at end of file From 11637127cb2ec30a7760ee7b73a4e01b85461d5c Mon Sep 17 00:00:00 2001 From: Oleksandr Shustov Date: Sun, 8 Nov 2020 20:01:47 +0200 Subject: [PATCH 4/4] remove redundant space --- CliFx/Domain/CommandArgumentSchema.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/CliFx/Domain/CommandArgumentSchema.cs b/CliFx/Domain/CommandArgumentSchema.cs index 0cd0cff..9f524a6 100644 --- a/CliFx/Domain/CommandArgumentSchema.cs +++ b/CliFx/Domain/CommandArgumentSchema.cs @@ -26,7 +26,6 @@ namespace CliFx.Domain Property = property; Description = description; ConverterType = converterType; - ValidatorTypes = validators; }