From 6b5718262652e2256850d3617f8ce532369a31c0 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Wed, 3 Feb 2021 14:13:18 -0800 Subject: [PATCH 1/9] Add Options Validation on Start --- .../tests/FakeOptions.cs | 5 +- ...ionsBuidlerConfigurationExtensionsTests.cs | 5 +- .../ref/Microsoft.Extensions.Options.cs | 4 + .../src/Microsoft.Extensions.Options.csproj | 1 + .../src/OptionsBuilderValidationExtensions.cs | 41 ++ .../src/ValidationHostedService.cs | 58 +++ .../src/ValidatorOptions.cs | 20 + .../AnnotatedOptions.cs | 27 + .../DepValidatorAttribute.cs | 29 ++ .../FromAttribute.cs | 17 + .../Microsoft.Extensions.Options.Tests.csproj | 1 + .../OptionsBuilderTest.cs | 47 -- .../OptionsBuilderValidationTests.cs | 467 ++++++++++++++++++ 13 files changed, 673 insertions(+), 49 deletions(-) create mode 100644 src/libraries/Microsoft.Extensions.Options/src/OptionsBuilderValidationExtensions.cs create mode 100644 src/libraries/Microsoft.Extensions.Options/src/ValidationHostedService.cs create mode 100644 src/libraries/Microsoft.Extensions.Options/src/ValidatorOptions.cs create mode 100644 src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/AnnotatedOptions.cs create mode 100644 src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/DepValidatorAttribute.cs create mode 100644 src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/FromAttribute.cs create mode 100644 src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsBuilderValidationTests.cs diff --git a/src/libraries/Microsoft.Extensions.Options.ConfigurationExtensions/tests/FakeOptions.cs b/src/libraries/Microsoft.Extensions.Options.ConfigurationExtensions/tests/FakeOptions.cs index 5a1b3d6144276b..7603db47c5ad7c 100644 --- a/src/libraries/Microsoft.Extensions.Options.ConfigurationExtensions/tests/FakeOptions.cs +++ b/src/libraries/Microsoft.Extensions.Options.ConfigurationExtensions/tests/FakeOptions.cs @@ -1,4 +1,7 @@ -namespace Microsoft.Extensions.Options.ConfigurationExtensions.Tests +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.Extensions.Options.ConfigurationExtensions.Tests { public class FakeOptions { diff --git a/src/libraries/Microsoft.Extensions.Options.ConfigurationExtensions/tests/OptionsBuidlerConfigurationExtensionsTests.cs b/src/libraries/Microsoft.Extensions.Options.ConfigurationExtensions/tests/OptionsBuidlerConfigurationExtensionsTests.cs index 2aa44c5fcd23ef..0aa34a9650f4a1 100644 --- a/src/libraries/Microsoft.Extensions.Options.ConfigurationExtensions/tests/OptionsBuidlerConfigurationExtensionsTests.cs +++ b/src/libraries/Microsoft.Extensions.Options.ConfigurationExtensions/tests/OptionsBuidlerConfigurationExtensionsTests.cs @@ -1,4 +1,7 @@ -using System; +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; using System.Collections.Generic; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; diff --git a/src/libraries/Microsoft.Extensions.Options/ref/Microsoft.Extensions.Options.cs b/src/libraries/Microsoft.Extensions.Options/ref/Microsoft.Extensions.Options.cs index d0b8af0aec10d1..5c583c77169093 100644 --- a/src/libraries/Microsoft.Extensions.Options/ref/Microsoft.Extensions.Options.cs +++ b/src/libraries/Microsoft.Extensions.Options/ref/Microsoft.Extensions.Options.cs @@ -6,6 +6,10 @@ namespace Microsoft.Extensions.DependencyInjection { + public static partial class OptionsBuilderValidationExtensions + { + public static Microsoft.Extensions.Options.OptionsBuilder ValidateOnStart(this Microsoft.Extensions.Options.OptionsBuilder optionsBuilder) where TOptions : class, new() { throw null; } + } public static partial class OptionsServiceCollectionExtensions { public static Microsoft.Extensions.DependencyInjection.IServiceCollection AddOptions(this Microsoft.Extensions.DependencyInjection.IServiceCollection services) { throw null; } diff --git a/src/libraries/Microsoft.Extensions.Options/src/Microsoft.Extensions.Options.csproj b/src/libraries/Microsoft.Extensions.Options/src/Microsoft.Extensions.Options.csproj index ba7dcec688a67e..bcb9bdd0a34f88 100644 --- a/src/libraries/Microsoft.Extensions.Options/src/Microsoft.Extensions.Options.csproj +++ b/src/libraries/Microsoft.Extensions.Options/src/Microsoft.Extensions.Options.csproj @@ -14,6 +14,7 @@ + diff --git a/src/libraries/Microsoft.Extensions.Options/src/OptionsBuilderValidationExtensions.cs b/src/libraries/Microsoft.Extensions.Options/src/OptionsBuilderValidationExtensions.cs new file mode 100644 index 00000000000000..7ee1476e147aec --- /dev/null +++ b/src/libraries/Microsoft.Extensions.Options/src/OptionsBuilderValidationExtensions.cs @@ -0,0 +1,41 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using Microsoft.Extensions.Options; + +namespace Microsoft.Extensions.DependencyInjection +{ + /// + /// Extension methods for adding configuration related options services to the DI container via . + /// + public static class OptionsBuilderValidationExtensions + { + /// + /// Enforces options validation check in startup time rather then in runtime. + /// + /// The type of options. + /// The to configure options instance. + /// The so that additional calls can be chained. + public static OptionsBuilder ValidateOnStart(this OptionsBuilder optionsBuilder) + where TOptions : class, new() + { + _ = optionsBuilder ?? throw new ArgumentNullException(nameof(optionsBuilder)); + + // This will only add the hosted service once + _ = optionsBuilder.Services.AddHostedService(); + + _ = optionsBuilder + .Services + .AddOptions() + .Configure>((vo, options) => + { + // This adds an action that resolves the options value to force evaluation + // We don't care about the result as duplicates aren't important + _ = vo.Validators.TryAdd(typeof(TOptions), () => _ = options.Get(optionsBuilder.Name)); + }); + + return optionsBuilder; + } + } +} \ No newline at end of file diff --git a/src/libraries/Microsoft.Extensions.Options/src/ValidationHostedService.cs b/src/libraries/Microsoft.Extensions.Options/src/ValidationHostedService.cs new file mode 100644 index 00000000000000..02a021257e7417 --- /dev/null +++ b/src/libraries/Microsoft.Extensions.Options/src/ValidationHostedService.cs @@ -0,0 +1,58 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; +using System.Runtime.ExceptionServices; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Options; + +namespace Microsoft.Extensions.DependencyInjection +{ + internal class ValidationHostedService : IHostedService + { + private readonly IDictionary _validators; + + public ValidationHostedService(IOptions validatorOptions) + { + _validators = validatorOptions?.Value?.Validators ?? throw new ArgumentNullException(nameof(validatorOptions)); + } + + public Task StartAsync(CancellationToken cancellationToken) + { + var exceptions = new List(); + + foreach (var validate in _validators.Values) + { + try + { + // Execute the validation method and catch the validation error + validate(); + } + catch (OptionsValidationException ex) + { + exceptions.Add(ex); + } + } + + if (exceptions.Count == 1) + { + // Rethrow if it's a single error + throw exceptions[0]; + } + + if (exceptions.Count > 1) + { + // Aggregate if we have many errors + throw new AggregateException(exceptions); + } + + return Task.CompletedTask; + } + + public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask; + } +} \ No newline at end of file diff --git a/src/libraries/Microsoft.Extensions.Options/src/ValidatorOptions.cs b/src/libraries/Microsoft.Extensions.Options/src/ValidatorOptions.cs new file mode 100644 index 00000000000000..137dbce98e4005 --- /dev/null +++ b/src/libraries/Microsoft.Extensions.Options/src/ValidatorOptions.cs @@ -0,0 +1,20 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using Microsoft.Extensions.Options; + +namespace Microsoft.Extensions.DependencyInjection +{ + internal class ValidatorOptions + { + // The key maps to the TOptions in IOptions and the value is a method + // that accesses the IOptions.Value property in order to force evaluation of + // the options type. + public ConcurrentDictionary Validators { get; } = new ConcurrentDictionary(); + } +} \ No newline at end of file diff --git a/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/AnnotatedOptions.cs b/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/AnnotatedOptions.cs new file mode 100644 index 00000000000000..4225282168c4bc --- /dev/null +++ b/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/AnnotatedOptions.cs @@ -0,0 +1,27 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.ComponentModel.DataAnnotations; +using System; + +namespace Microsoft.Extensions.Options.Tests +{ + public class AnnotatedOptions + { + [Required] + public string Required { get; set; } + + [StringLength(5, ErrorMessage = "Too long.")] + public string StringLength { get; set; } + + [Range(-5, 5, ErrorMessage = "Out of range.")] + public int IntRange { get; set; } + + [From(Accepted = "USA")] + public string Custom { get; set; } + + [DepValidator(Target = "Dep2")] + public string Dep1 { get; set; } + public string Dep2 { get; set; } + } +} diff --git a/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/DepValidatorAttribute.cs b/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/DepValidatorAttribute.cs new file mode 100644 index 00000000000000..330e16f13a033d --- /dev/null +++ b/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/DepValidatorAttribute.cs @@ -0,0 +1,29 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.ComponentModel.DataAnnotations; +using System; + +namespace Microsoft.Extensions.Options.Tests +{ + [AttributeUsage(AttributeTargets.Field | AttributeTargets.Property, AllowMultiple = false, Inherited = true)] + public sealed class DepValidatorAttribute + : ValidationAttribute + { + public string Target { get; set; } + + protected override ValidationResult IsValid(object value, ValidationContext validationContext) + { + object instance = validationContext.ObjectInstance; + Type type = instance.GetType(); + var dep1 = type.GetProperty("Dep1")?.GetValue(instance); + var dep2 = type.GetProperty(Target)?.GetValue(instance); + if (dep1 == dep2) + { + return ValidationResult.Success; + } + + return new ValidationResult("Dep1 != " + Target, new[] { "Dep1", Target }); + } + } +} diff --git a/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/FromAttribute.cs b/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/FromAttribute.cs new file mode 100644 index 00000000000000..f16e7f0726baac --- /dev/null +++ b/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/FromAttribute.cs @@ -0,0 +1,17 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.ComponentModel.DataAnnotations; +using System; + +namespace Microsoft.Extensions.Options.Tests +{ + [AttributeUsage(AttributeTargets.Field | AttributeTargets.Property, AllowMultiple = false, Inherited = true)] + public sealed class FromAttribute : ValidationAttribute + { + public string Accepted { get; set; } + + public override bool IsValid(object value) + => value == null || value.ToString() == Accepted; + } +} diff --git a/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/Microsoft.Extensions.Options.Tests.csproj b/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/Microsoft.Extensions.Options.Tests.csproj index e6fb72af73171f..f9e8426555e305 100644 --- a/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/Microsoft.Extensions.Options.Tests.csproj +++ b/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/Microsoft.Extensions.Options.Tests.csproj @@ -8,6 +8,7 @@ + diff --git a/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsBuilderTest.cs b/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsBuilderTest.cs index 59514ea56360d4..86adf09623399f 100644 --- a/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsBuilderTest.cs +++ b/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsBuilderTest.cs @@ -567,53 +567,6 @@ public void CanValidateOptionsEagerly() ValidateFailure(error, Options.DefaultName, 3, "A validation error has occurred.", "Virtual", "Integer"); } - [AttributeUsage(AttributeTargets.Field | AttributeTargets.Property, AllowMultiple = false, Inherited = true)] - public class FromAttribute : ValidationAttribute - { - public string Accepted { get; set; } - - public override bool IsValid(object value) - => value == null || value.ToString() == Accepted; - } - - [AttributeUsage(AttributeTargets.Field | AttributeTargets.Property, AllowMultiple = false, Inherited = true)] - public class DepValidator : ValidationAttribute - { - public string Target { get; set; } - - protected override ValidationResult IsValid(object value, ValidationContext validationContext) - { - object instance = validationContext.ObjectInstance; - Type type = instance.GetType(); - var dep1 = type.GetProperty("Dep1")?.GetValue(instance); - var dep2 = type.GetProperty(Target)?.GetValue(instance); - if (dep1 == dep2) - { - return ValidationResult.Success; - } - return new ValidationResult("Dep1 != "+Target, new string[] { "Dep1", Target }); - } - } - - private class AnnotatedOptions - { - [Required] - public string Required { get; set; } - - [StringLength(5, ErrorMessage = "Too long.")] - public string StringLength { get; set; } - - [Range(-5, 5, ErrorMessage = "Out of range.")] - public int IntRange { get; set; } - - [From(Accepted = "USA")] - public string Custom { get; set; } - - [DepValidator(Target = "Dep2")] - public string Dep1 { get; set; } - public string Dep2 { get; set; } - } - [Fact] public void CanValidateDataAnnotations() { diff --git a/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsBuilderValidationTests.cs b/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsBuilderValidationTests.cs new file mode 100644 index 00000000000000..d944597434fd60 --- /dev/null +++ b/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsBuilderValidationTests.cs @@ -0,0 +1,467 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Linq; +using System.Threading.Tasks; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Options.Tests; +using Xunit; + +namespace Microsoft.Extensions.Options.DataAnnotations.Tests +{ + public class OptionsBuilderValidationTests + { + public static IHostBuilder CreateHostBuilder(Action configure) + { + return new HostBuilder().ConfigureServices(configure); + } + + [Fact] + public async Task CanValidateOptionsOnStartWithDefaultError() + { + var hostBuilder = CreateHostBuilder(services => + { + services.AddOptions() + .Configure(o => o.Boolean = false) + .Validate(o => o.Boolean) + .ValidateOnStart(); + }); + + using (var host = hostBuilder.Build()) + { + var error = await Assert.ThrowsAsync(async () => + { + await host.StartAsync(); + }); + + ValidateFailure(error); + } + } + + [Fact] + public async Task CanValidateOptionsOnStartWithCustomError() + { + var hostBuilder = CreateHostBuilder(services => + { + services.AddOptions() + .Configure(o => o.Boolean = false) + .Validate(o => o.Boolean, "first Boolean must be true.") + .ValidateOnStart(); + services.AddOptions() + .Configure(o => o.Boolean = true) + .Validate(o => !o.Boolean, "second Boolean must be false.") + .ValidateOnStart(); + }); + + using (var host = hostBuilder.Build()) + { + var error = await Assert.ThrowsAsync(async () => + { + await host.StartAsync(); + }); + + ValidateFailure(error, 1, "second Boolean must be false."); + } + } + + [Fact] + public async Task CanValidateOptionsOnStartRatherThanLazySameType() // shouldn this fail? + { + var hostBuilder = CreateHostBuilder(services => + { + services.AddOptions() + .Configure(o => o.Boolean = false) + .Validate(o => o.Boolean, "first Boolean must be true.") + .ValidateOnStart(); + services.AddOptions() + .Configure(o => o.Boolean = true) + .Validate(o => !o.Boolean, "second Boolean must be false."); + }); + + using (var host = hostBuilder.Build()) + { + var error = await Assert.ThrowsAsync(async () => + { + await host.StartAsync(); + }); + + ValidateFailure(error, 1, "second Boolean must be false."); + } + } + + [Fact] + public async Task CanValidateOptionsLazyThanEagerSameType() + { + var hostBuilder = CreateHostBuilder(services => + { + services.AddOptions() + .Configure(o => o.Boolean = false) + .Validate(o => o.Boolean, "first Boolean must be true."); + services.AddOptions() + .Configure(o => o.Boolean = true) + .Validate(o => !o.Boolean, "second Boolean must be false.") + .ValidateOnStart(); + }); + + using (var host = hostBuilder.Build()) + { + var error = await Assert.ThrowsAsync(async () => + { + await host.StartAsync(); + }); + + ValidateFailure(error, 1, "second Boolean must be false."); + } + } + + [Fact] + public async Task CanValidateOptionsLazyThanEagerDifferentTypes() + { + var hostBuilder = CreateHostBuilder(services => + { + services.AddOptions() + .Configure(o => o.Integer = 11) + .Validate(o => o.Integer > 12, "Integer"); + + services.AddOptions() + .Configure(o => o.Boolean = false) + .Validate(o => o.Boolean, "first Boolean must be true.") + .ValidateOnStart(); + }); + + using (var host = hostBuilder.Build()) + { + var error = await Assert.ThrowsAsync(async () => + { + await host.StartAsync(); + }); + + ValidateFailure(error, 1, "first Boolean must be true."); + } + } + + [Fact] + public async Task CanValidateOptionsOnStartAndSomeLazyDifferentTypes() + { + var hostBuilder = CreateHostBuilder(services => + { + services.AddOptions() + .Configure(o => o.Integer = 11) + .Validate(o => o.Integer > 12, "Integer") + .ValidateOnStart(); + + services.AddOptions() + .Configure(o => o.Boolean = false) + .Validate(o => o.Boolean, "first Boolean must be true."); + }); + + using (var host = hostBuilder.Build()) + { + var error = await Assert.ThrowsAsync(async () => + { + await host.StartAsync(); + }); + + ValidateFailure(error, 1, "Integer"); + } + } + + [Fact] + public async Task CanValidateOptionsOnStartWithMultipleDefaultErrors() + { + var hostBuilder = CreateHostBuilder(services => + { + services.AddOptions() + .Configure(o => + { + o.Boolean = false; + o.Integer = 11; + }) + .Validate(o => o.Boolean) + .Validate(o => o.Integer > 12) + .ValidateOnStart(); + }); + + using (var host = hostBuilder.Build()) + { + var error = await Assert.ThrowsAsync(async () => + { + await host.StartAsync(); + }); + + ValidateFailure(error, 2); + } + } + + [Fact] + public async Task CanValidateOptionOnStartsWithMixedOverloads() + { + var hostBuilder = CreateHostBuilder(services => + { + services.AddOptions() + .Configure(o => + { + o.Boolean = false; + o.Integer = 11; + o.Virtual = "wut"; + }) + .Validate(o => o.Boolean) + .Validate(o => o.Virtual == null, "Virtual") + .Validate(o => o.Integer > 12, "Integer") + .ValidateOnStart(); + }); + using (var host = hostBuilder.Build()) + { + var error = await Assert.ThrowsAsync(async () => + { + await host.StartAsync(); + }); + + ValidateFailure(error, 3, "Virtual", "Integer"); + } + } + + [Fact] + public async Task CanValidateOnStartDataAnnotationsLongSyntax() + { + var hostBuilder = CreateHostBuilder(services => + { + services.AddOptions() + .Configure(o => + { + o.StringLength = "111111"; + o.IntRange = 10; + o.Custom = "nowhere"; + o.Dep1 = "Not dep2"; + }) + .ValidateDataAnnotations() + .ValidateOnStart(); + }); + + using (var host = hostBuilder.Build()) + { + var error = await Assert.ThrowsAsync(async () => + { + await host.StartAsync(); + }); + + ValidateFailure(error, 5, + "DataAnnotation validation failed for members: 'Required' with the error: 'The Required field is required.'.", + "DataAnnotation validation failed for members: 'StringLength' with the error: 'Too long.'.", + "DataAnnotation validation failed for members: 'IntRange' with the error: 'Out of range.'.", + "DataAnnotation validation failed for members: 'Custom' with the error: 'The field Custom is invalid.'.", + "DataAnnotation validation failed for members: 'Dep1,Dep2' with the error: 'Dep1 != Dep2'."); + } + } + + [Fact] + public async Task CanValidateOnStartMixDataAnnotationsLongSyntax() + { + var hostBuilder = CreateHostBuilder(services => + { + services.AddOptions() + .Configure(o => + { + o.StringLength = "111111"; + o.IntRange = 10; + o.Custom = "nowhere"; + o.Dep1 = "Not dep2"; + }) + .ValidateDataAnnotations() + .Validate(o => o.Custom != "nowhere", "I don't want to go to nowhere!") + .ValidateOnStart(); + }); + + using (var host = hostBuilder.Build()) + { + var error = await Assert.ThrowsAsync(async () => + { + await host.StartAsync(); + }); + + ValidateFailure(error, 6, + "DataAnnotation validation failed for members: 'Required' with the error: 'The Required field is required.'.", + "DataAnnotation validation failed for members: 'StringLength' with the error: 'Too long.'.", + "DataAnnotation validation failed for members: 'IntRange' with the error: 'Out of range.'.", + "DataAnnotation validation failed for members: 'Custom' with the error: 'The field Custom is invalid.'.", + "DataAnnotation validation failed for members: 'Dep1,Dep2' with the error: 'Dep1 != Dep2'.", + "I don't want to go to nowhere!"); + } + } + + [Fact] + public async Task CanValidateOnStartDataAnnotationsShortSyntax() + { + var hostBuilder = CreateHostBuilder(services => + { + services.AddOptions() + .Configure(o => + { + o.StringLength = "111111"; + o.IntRange = 10; + o.Custom = "nowhere"; + o.Dep1 = "Not dep2"; + }) + .ValidateDataAnnotations() + .ValidateOnStart(); + }); + + using (var host = hostBuilder.Build()) + { + var error = await Assert.ThrowsAsync(async () => + { + await host.StartAsync(); + }); + + ValidateFailure(error, 5, + "DataAnnotation validation failed for members: 'Required' with the error: 'The Required field is required.'.", + "DataAnnotation validation failed for members: 'StringLength' with the error: 'Too long.'.", + "DataAnnotation validation failed for members: 'IntRange' with the error: 'Out of range.'.", + "DataAnnotation validation failed for members: 'Custom' with the error: 'The field Custom is invalid.'.", + "DataAnnotation validation failed for members: 'Dep1,Dep2' with the error: 'Dep1 != Dep2'."); + } + } + + [Fact] + public async Task CanValidateOnStartMixDataAnnotationsShortSyntax() + { + var hostBuilder = CreateHostBuilder(services => + { + services.AddOptions() + .Configure(o => + { + o.StringLength = "111111"; + o.IntRange = 10; + o.Custom = "nowhere"; + o.Dep1 = "Not dep2"; + }) + .ValidateDataAnnotations() + .ValidateOnStart() + .Validate(o => o.Custom != "nowhere", "I don't want to go to nowhere!"); + }); + + using (var host = hostBuilder.Build()) + { + var error = await Assert.ThrowsAsync(async () => + { + await host.StartAsync(); + }); + + ValidateFailure(error, 6, + "DataAnnotation validation failed for members: 'Required' with the error: 'The Required field is required.'.", + "DataAnnotation validation failed for members: 'StringLength' with the error: 'Too long.'.", + "DataAnnotation validation failed for members: 'IntRange' with the error: 'Out of range.'.", + "DataAnnotation validation failed for members: 'Custom' with the error: 'The field Custom is invalid.'.", + "DataAnnotation validation failed for members: 'Dep1,Dep2' with the error: 'Dep1 != Dep2'.", + "I don't want to go to nowhere!"); + } + } + + [Fact] + public void Test_WhenValidateOnStartThrowsIfArgumentNull() + { + var hostBuilder = CreateHostBuilder(services => + { + OptionsBuilder optionsBuilder = null; +#pragma warning disable CS8604 // Possible null reference argument. + optionsBuilder.ValidateOnStart(); +#pragma warning restore CS8604 // Possible null reference argument. + }); + + _ = Assert.Throws(() => { _ = hostBuilder.Build(); }); + } + + internal class FakeService { } + + internal class FakeSettings + { + public string Name { get; set; } + } + + [Fact] + public async Task ValidateOnStart_NamedOptions_FailsOnStartForFailedValidation() + { + var hostBuilder = CreateHostBuilder(services => + { + services.AddOptions().AddSingleton(new FakeService()); + services + .AddOptions("named") + .Configure((o, _) => + { + o.Name = "named"; + }) + .Validate(o => o.Name == null, "trigger validation failure for named option!") + .ValidateOnStart(); + }); + + using (var host = hostBuilder.Build()) + { + var error = await Assert.ThrowsAsync(async () => + { + await host.StartAsync(); + }); + + ValidateFailure(error, 1, "trigger validation failure for named option!"); + } + } + + [Fact] + public async Task Test_IVfalidationSuccessful() + { + var hostBuilder = CreateHostBuilder(services => + { + services.AddOptions() + .Configure(o => + { + o.Required = "required"; + o.StringLength = "1111"; + o.IntRange = 0; + o.Custom = "USA"; + o.Dep1 = "dep"; + o.Dep2 = "dep"; + }) + .ValidateDataAnnotations() + .ValidateOnStart() + .Validate(o => o.Custom != "nowhere", "I don't want to go to nowhere!"); + }); + + using (var host = hostBuilder.Build()) + { + try + { + await host.StartAsync(); + } + #pragma warning disable CA1031 // Do not catch general exception types + catch (Exception ex) + #pragma warning restore CA1031 // Do not catch general exception types + { + Assert.True(false, "Expected no exception, but got: " + ex.Message); + } + } + } + + private static void ValidateFailure(Type type, OptionsValidationException e, int count = 1, params string[] errorsToMatch) + { + Assert.Equal(type, e.OptionsType); + + Assert.Equal(count, e.Failures.Count()); + + // Check for the error in any of the failures + foreach (var error in errorsToMatch) + { +#if NETCOREAPP + Assert.True(e.Failures.FirstOrDefault(predicate: f => f.Contains(error, StringComparison.CurrentCulture)) != null, "Did not find: " + error); +#else + Assert.True(e.Failures.FirstOrDefault(predicate: f => f.IndexOf(error, StringComparison.CurrentCulture) >= 0) != null, "Did not find: " + error); +#endif + } + } + + private static void ValidateFailure(OptionsValidationException e, int count = 1, params string[] errorsToMatch) + { + ValidateFailure(typeof(TOptions), e, count, errorsToMatch); + } + } +} From 2d6a5d979080c7f0eef786b462585ea54a8c34ed Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Thu, 4 Feb 2021 12:11:58 -0800 Subject: [PATCH 2/9] Apply PR feedback: - Drop new() constraint - Remove _ in most cases - Remove the ConcurrentDictionary - Slight comment cleanup - Skip tests on mono: due to PNSE on host.StartAsync() --- .../ref/Microsoft.Extensions.Options.cs | 2 +- .../src/OptionsBuilderValidationExtensions.cs | 31 +++++++++---------- .../src/ValidationHostedService.cs | 3 +- .../src/ValidatorOptions.cs | 12 ++----- .../OptionsBuilderValidationTests.cs | 23 ++++++++++---- 5 files changed, 37 insertions(+), 34 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Options/ref/Microsoft.Extensions.Options.cs b/src/libraries/Microsoft.Extensions.Options/ref/Microsoft.Extensions.Options.cs index 5c583c77169093..6f0242b508b9c0 100644 --- a/src/libraries/Microsoft.Extensions.Options/ref/Microsoft.Extensions.Options.cs +++ b/src/libraries/Microsoft.Extensions.Options/ref/Microsoft.Extensions.Options.cs @@ -8,7 +8,7 @@ namespace Microsoft.Extensions.DependencyInjection { public static partial class OptionsBuilderValidationExtensions { - public static Microsoft.Extensions.Options.OptionsBuilder ValidateOnStart(this Microsoft.Extensions.Options.OptionsBuilder optionsBuilder) where TOptions : class, new() { throw null; } + public static Microsoft.Extensions.Options.OptionsBuilder ValidateOnStart(this Microsoft.Extensions.Options.OptionsBuilder optionsBuilder) where TOptions : class { throw null; } } public static partial class OptionsServiceCollectionExtensions { diff --git a/src/libraries/Microsoft.Extensions.Options/src/OptionsBuilderValidationExtensions.cs b/src/libraries/Microsoft.Extensions.Options/src/OptionsBuilderValidationExtensions.cs index 7ee1476e147aec..a5698a22d4d79b 100644 --- a/src/libraries/Microsoft.Extensions.Options/src/OptionsBuilderValidationExtensions.cs +++ b/src/libraries/Microsoft.Extensions.Options/src/OptionsBuilderValidationExtensions.cs @@ -12,30 +12,29 @@ namespace Microsoft.Extensions.DependencyInjection public static class OptionsBuilderValidationExtensions { /// - /// Enforces options validation check in startup time rather then in runtime. + /// Enforces options validation check on start rather then in runtime. /// /// The type of options. /// The to configure options instance. /// The so that additional calls can be chained. public static OptionsBuilder ValidateOnStart(this OptionsBuilder optionsBuilder) - where TOptions : class, new() + where TOptions : class { - _ = optionsBuilder ?? throw new ArgumentNullException(nameof(optionsBuilder)); + if (optionsBuilder == null) + { + throw new ArgumentNullException(nameof(optionsBuilder)); + } - // This will only add the hosted service once - _ = optionsBuilder.Services.AddHostedService(); - - _ = optionsBuilder - .Services - .AddOptions() - .Configure>((vo, options) => - { - // This adds an action that resolves the options value to force evaluation - // We don't care about the result as duplicates aren't important - _ = vo.Validators.TryAdd(typeof(TOptions), () => _ = options.Get(optionsBuilder.Name)); - }); + optionsBuilder.Services.AddHostedService(); + optionsBuilder.Services.AddOptions() + .Configure>((vo, options) => + { + // This adds an action that resolves the options value to force evaluation + // We don't care about the result as duplicates are not important + vo.Validators[typeof(TOptions)] = () => options.Get(optionsBuilder.Name); + }); return optionsBuilder; } } -} \ No newline at end of file +} diff --git a/src/libraries/Microsoft.Extensions.Options/src/ValidationHostedService.cs b/src/libraries/Microsoft.Extensions.Options/src/ValidationHostedService.cs index 02a021257e7417..f6b2b319554404 100644 --- a/src/libraries/Microsoft.Extensions.Options/src/ValidationHostedService.cs +++ b/src/libraries/Microsoft.Extensions.Options/src/ValidationHostedService.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using System.Diagnostics.CodeAnalysis; using System.Runtime.ExceptionServices; using System.Threading; using System.Threading.Tasks; @@ -41,7 +40,7 @@ public Task StartAsync(CancellationToken cancellationToken) if (exceptions.Count == 1) { // Rethrow if it's a single error - throw exceptions[0]; + ExceptionDispatchInfo.Capture(exceptions[0]).Throw(); } if (exceptions.Count > 1) diff --git a/src/libraries/Microsoft.Extensions.Options/src/ValidatorOptions.cs b/src/libraries/Microsoft.Extensions.Options/src/ValidatorOptions.cs index 137dbce98e4005..9701860396de4c 100644 --- a/src/libraries/Microsoft.Extensions.Options/src/ValidatorOptions.cs +++ b/src/libraries/Microsoft.Extensions.Options/src/ValidatorOptions.cs @@ -2,19 +2,13 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; -using System.Collections.Concurrent; using System.Collections.Generic; -using System.Linq; -using System.Text; -using Microsoft.Extensions.Options; namespace Microsoft.Extensions.DependencyInjection { internal class ValidatorOptions { - // The key maps to the TOptions in IOptions and the value is a method - // that accesses the IOptions.Value property in order to force evaluation of - // the options type. - public ConcurrentDictionary Validators { get; } = new ConcurrentDictionary(); + // Maps each options type to a method that forces its evaluation, e.g. IOptionsMonitor.Get(name) + public IDictionary Validators { get; } = new Dictionary(); } -} \ No newline at end of file +} diff --git a/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsBuilderValidationTests.cs b/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsBuilderValidationTests.cs index d944597434fd60..5b9e9df7b6dac6 100644 --- a/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsBuilderValidationTests.cs +++ b/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsBuilderValidationTests.cs @@ -6,10 +6,9 @@ using System.Threading.Tasks; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; -using Microsoft.Extensions.Options.Tests; using Xunit; -namespace Microsoft.Extensions.Options.DataAnnotations.Tests +namespace Microsoft.Extensions.Options.Tests { public class OptionsBuilderValidationTests { @@ -19,6 +18,7 @@ public static IHostBuilder CreateHostBuilder(Action configur } [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] public async Task CanValidateOptionsOnStartWithDefaultError() { var hostBuilder = CreateHostBuilder(services => @@ -41,6 +41,7 @@ public async Task CanValidateOptionsOnStartWithDefaultError() } [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] public async Task CanValidateOptionsOnStartWithCustomError() { var hostBuilder = CreateHostBuilder(services => @@ -67,6 +68,7 @@ public async Task CanValidateOptionsOnStartWithCustomError() } [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] public async Task CanValidateOptionsOnStartRatherThanLazySameType() // shouldn this fail? { var hostBuilder = CreateHostBuilder(services => @@ -92,6 +94,7 @@ public async Task CanValidateOptionsOnStartRatherThanLazySameType() // shouldn t } [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] public async Task CanValidateOptionsLazyThanEagerSameType() { var hostBuilder = CreateHostBuilder(services => @@ -117,6 +120,7 @@ public async Task CanValidateOptionsLazyThanEagerSameType() } [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] public async Task CanValidateOptionsLazyThanEagerDifferentTypes() { var hostBuilder = CreateHostBuilder(services => @@ -143,6 +147,7 @@ public async Task CanValidateOptionsLazyThanEagerDifferentTypes() } [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] public async Task CanValidateOptionsOnStartAndSomeLazyDifferentTypes() { var hostBuilder = CreateHostBuilder(services => @@ -169,6 +174,7 @@ public async Task CanValidateOptionsOnStartAndSomeLazyDifferentTypes() } [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] public async Task CanValidateOptionsOnStartWithMultipleDefaultErrors() { var hostBuilder = CreateHostBuilder(services => @@ -196,6 +202,7 @@ public async Task CanValidateOptionsOnStartWithMultipleDefaultErrors() } [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] public async Task CanValidateOptionOnStartsWithMixedOverloads() { var hostBuilder = CreateHostBuilder(services => @@ -224,6 +231,7 @@ public async Task CanValidateOptionOnStartsWithMixedOverloads() } [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] public async Task CanValidateOnStartDataAnnotationsLongSyntax() { var hostBuilder = CreateHostBuilder(services => @@ -257,6 +265,7 @@ public async Task CanValidateOnStartDataAnnotationsLongSyntax() } [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] public async Task CanValidateOnStartMixDataAnnotationsLongSyntax() { var hostBuilder = CreateHostBuilder(services => @@ -292,6 +301,7 @@ public async Task CanValidateOnStartMixDataAnnotationsLongSyntax() } [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] public async Task CanValidateOnStartDataAnnotationsShortSyntax() { var hostBuilder = CreateHostBuilder(services => @@ -325,6 +335,7 @@ public async Task CanValidateOnStartDataAnnotationsShortSyntax() } [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] public async Task CanValidateOnStartMixDataAnnotationsShortSyntax() { var hostBuilder = CreateHostBuilder(services => @@ -360,17 +371,15 @@ public async Task CanValidateOnStartMixDataAnnotationsShortSyntax() } [Fact] - public void Test_WhenValidateOnStartThrowsIfArgumentNull() + public void ValidateOnStart_NullOptionsBuilder_Throws() { var hostBuilder = CreateHostBuilder(services => { OptionsBuilder optionsBuilder = null; -#pragma warning disable CS8604 // Possible null reference argument. optionsBuilder.ValidateOnStart(); -#pragma warning restore CS8604 // Possible null reference argument. }); - _ = Assert.Throws(() => { _ = hostBuilder.Build(); }); + Assert.Throws(() => hostBuilder.Build()); } internal class FakeService { } @@ -381,6 +390,7 @@ internal class FakeSettings } [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] public async Task ValidateOnStart_NamedOptions_FailsOnStartForFailedValidation() { var hostBuilder = CreateHostBuilder(services => @@ -408,6 +418,7 @@ public async Task ValidateOnStart_NamedOptions_FailsOnStartForFailedValidation() } [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] public async Task Test_IVfalidationSuccessful() { var hostBuilder = CreateHostBuilder(services => From 68e1b66345d8e4d8174f61c892201f76a18331b9 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Thu, 4 Feb 2021 19:08:50 -0800 Subject: [PATCH 3/9] Cleanup test class --- .../OptionsBuilderValidationTests.cs | 229 ++++++++---------- 1 file changed, 102 insertions(+), 127 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsBuilderValidationTests.cs b/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsBuilderValidationTests.cs index 5b9e9df7b6dac6..64aca348fc66e0 100644 --- a/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsBuilderValidationTests.cs +++ b/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsBuilderValidationTests.cs @@ -17,9 +17,21 @@ public static IHostBuilder CreateHostBuilder(Action configur return new HostBuilder().ConfigureServices(configure); } + [Fact] + public void ValidateOnStart_NullOptionsBuilder_Throws() + { + var hostBuilder = CreateHostBuilder(services => + { + OptionsBuilder optionsBuilder = null; + optionsBuilder.ValidateOnStart(); + }); + + Assert.Throws(() => hostBuilder.Build()); + } + [Fact] [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] - public async Task CanValidateOptionsOnStartWithDefaultError() + public async Task ValidateOnStart_ConfigureAndValidateThenCallValidateOnStart_ValidatesFailure() { var hostBuilder = CreateHostBuilder(services => { @@ -36,24 +48,20 @@ public async Task CanValidateOptionsOnStartWithDefaultError() await host.StartAsync(); }); - ValidateFailure(error); + ValidateFailure(error, 1); } } [Fact] [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] - public async Task CanValidateOptionsOnStartWithCustomError() + public async Task ValidateOnStart_CallFirstThenConfigureAndValidate_ValidatesFailure() { var hostBuilder = CreateHostBuilder(services => { services.AddOptions() + .ValidateOnStart() .Configure(o => o.Boolean = false) - .Validate(o => o.Boolean, "first Boolean must be true.") - .ValidateOnStart(); - services.AddOptions() - .Configure(o => o.Boolean = true) - .Validate(o => !o.Boolean, "second Boolean must be false.") - .ValidateOnStart(); + .Validate(o => o.Boolean); }); using (var host = hostBuilder.Build()) @@ -63,23 +71,20 @@ public async Task CanValidateOptionsOnStartWithCustomError() await host.StartAsync(); }); - ValidateFailure(error, 1, "second Boolean must be false."); + ValidateFailure(error, 1); } } [Fact] [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] - public async Task CanValidateOptionsOnStartRatherThanLazySameType() // shouldn this fail? + public async Task ValidateOnStart_ErrorMessageSpecified_FailsWithCustomError() { var hostBuilder = CreateHostBuilder(services => { services.AddOptions() .Configure(o => o.Boolean = false) - .Validate(o => o.Boolean, "first Boolean must be true.") + .Validate(o => o.Boolean, "Boolean must be true.") .ValidateOnStart(); - services.AddOptions() - .Configure(o => o.Boolean = true) - .Validate(o => !o.Boolean, "second Boolean must be false."); }); using (var host = hostBuilder.Build()) @@ -89,22 +94,31 @@ public async Task CanValidateOptionsOnStartRatherThanLazySameType() // shouldn t await host.StartAsync(); }); - ValidateFailure(error, 1, "second Boolean must be false."); + ValidateFailure(error, 1, "Boolean must be true."); } } + internal class FakeService { } + + internal class FakeSettings + { + public string Name { get; set; } + } + [Fact] [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] - public async Task CanValidateOptionsLazyThanEagerSameType() + public async Task ValidateOnStart_NamedOptions_ValidatesFailureOnStart() { var hostBuilder = CreateHostBuilder(services => { - services.AddOptions() - .Configure(o => o.Boolean = false) - .Validate(o => o.Boolean, "first Boolean must be true."); - services.AddOptions() - .Configure(o => o.Boolean = true) - .Validate(o => !o.Boolean, "second Boolean must be false.") + services.AddOptions().AddSingleton(new FakeService()); + services + .AddOptions("named") + .Configure((o, _) => + { + o.Name = "named"; + }) + .Validate(o => o.Name == null, "trigger validation failure for named option!") .ValidateOnStart(); }); @@ -115,23 +129,29 @@ public async Task CanValidateOptionsLazyThanEagerSameType() await host.StartAsync(); }); - ValidateFailure(error, 1, "second Boolean must be false."); + ValidateFailure(error, 1, "trigger validation failure for named option!"); } } [Fact] [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] - public async Task CanValidateOptionsLazyThanEagerDifferentTypes() + private async Task ValidateOnStart_AddOptionsMultipleTimesForSameType_LastValidateOnStartCallGetsTriggered() { var hostBuilder = CreateHostBuilder(services => { - services.AddOptions() - .Configure(o => o.Integer = 11) - .Validate(o => o.Integer > 12, "Integer"); - - services.AddOptions() + services.AddOptions("bad_configuration1") .Configure(o => o.Boolean = false) - .Validate(o => o.Boolean, "first Boolean must be true.") + .Validate(o => o.Boolean, "bad_configuration1") + .ValidateOnStart(); + + services.AddOptions("bad_configuration2") + .Configure(o => + { + o.Boolean = false; + o.Integer = 11; + }) + .Validate(o => o.Boolean, "Boolean") + .Validate(o => o.Integer > 12, "Integer") .ValidateOnStart(); }); @@ -142,24 +162,60 @@ public async Task CanValidateOptionsLazyThanEagerDifferentTypes() await host.StartAsync(); }); - ValidateFailure(error, 1, "first Boolean must be true."); + ValidateFailure(error, 2, "Boolean", "Integer"); + } + } + + [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] + private async Task ValidateOnStart_AddBothLazyAndEagerValidationOnSameType_SkipsLazyValidationWhenHostStarts() + { + var hostBuilder = CreateHostBuilder(services => + { + // Adds eager validation using ValidateOnStart + services.AddOptions("correct_configuration") + .Configure(o => o.Boolean = true) + .Validate(o => o.Boolean, "correct_configuration") + .ValidateOnStart(); + + // Adds lazy validation, skipping validation on start + services.AddOptions("bad_configuration") + .Configure(o => o.Boolean = false) + .Validate(o => o.Boolean, "bad_configuration"); + }); + + // For the lazily added "bad_configuration", validation failure does not occur when host starts + using (var host = hostBuilder.Build()) + { + try + { + await host.StartAsync(); + } + #pragma warning disable CA1031 // Do not catch general exception types + catch (Exception ex) + #pragma warning restore CA1031 // Do not catch general exception types + { + Assert.True(false, "Expected no exception, but got: " + ex.Message); + } } } [Fact] [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] - public async Task CanValidateOptionsOnStartAndSomeLazyDifferentTypes() + public async Task ValidateOnStart_AddBothLazyAndEagerValidationOnDifferentTypes_SkipsLazyValidationWhenHostStarts() { var hostBuilder = CreateHostBuilder(services => { + // Lazy validation services.AddOptions() .Configure(o => o.Integer = 11) - .Validate(o => o.Integer > 12, "Integer") - .ValidateOnStart(); + .Validate(o => o.Integer > 12, "Integer"); + // Eager validation services.AddOptions() .Configure(o => o.Boolean = false) - .Validate(o => o.Boolean, "first Boolean must be true."); + .Validate(o => o.Boolean, "first Boolean must be true.") + .ValidateOnStart(); }); using (var host = hostBuilder.Build()) @@ -169,13 +225,13 @@ public async Task CanValidateOptionsOnStartAndSomeLazyDifferentTypes() await host.StartAsync(); }); - ValidateFailure(error, 1, "Integer"); + ValidateFailure(error, 1, "first Boolean must be true."); } } [Fact] [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] - public async Task CanValidateOptionsOnStartWithMultipleDefaultErrors() + public async Task ValidateOnStart_MultipleErrorsInOneValidationCall_ValidatesFailureWithMultipleErrors() { var hostBuilder = CreateHostBuilder(services => { @@ -203,7 +259,7 @@ public async Task CanValidateOptionsOnStartWithMultipleDefaultErrors() [Fact] [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] - public async Task CanValidateOptionOnStartsWithMixedOverloads() + public async Task ValidateOnStart_MultipleErrorsInOneValidationCallUsingCustomErrors_FailuresContainCustomErrors() { var hostBuilder = CreateHostBuilder(services => { @@ -232,7 +288,7 @@ public async Task CanValidateOptionOnStartsWithMixedOverloads() [Fact] [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] - public async Task CanValidateOnStartDataAnnotationsLongSyntax() + public async Task ValidateOnStart_CallValidateDataAnnotations_ValidationSuccessful() { var hostBuilder = CreateHostBuilder(services => { @@ -266,7 +322,7 @@ public async Task CanValidateOnStartDataAnnotationsLongSyntax() [Fact] [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] - public async Task CanValidateOnStartMixDataAnnotationsLongSyntax() + public async Task ValidateOnStart_CallValidateAndValidateDataAnnotations_FailuresCaughtFromBothValidateAndValidateDataAnnotations() { var hostBuilder = CreateHostBuilder(services => { @@ -302,45 +358,12 @@ public async Task CanValidateOnStartMixDataAnnotationsLongSyntax() [Fact] [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] - public async Task CanValidateOnStartDataAnnotationsShortSyntax() - { - var hostBuilder = CreateHostBuilder(services => - { - services.AddOptions() - .Configure(o => - { - o.StringLength = "111111"; - o.IntRange = 10; - o.Custom = "nowhere"; - o.Dep1 = "Not dep2"; - }) - .ValidateDataAnnotations() - .ValidateOnStart(); - }); - - using (var host = hostBuilder.Build()) - { - var error = await Assert.ThrowsAsync(async () => - { - await host.StartAsync(); - }); - - ValidateFailure(error, 5, - "DataAnnotation validation failed for members: 'Required' with the error: 'The Required field is required.'.", - "DataAnnotation validation failed for members: 'StringLength' with the error: 'Too long.'.", - "DataAnnotation validation failed for members: 'IntRange' with the error: 'Out of range.'.", - "DataAnnotation validation failed for members: 'Custom' with the error: 'The field Custom is invalid.'.", - "DataAnnotation validation failed for members: 'Dep1,Dep2' with the error: 'Dep1 != Dep2'."); - } - } - - [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] - public async Task CanValidateOnStartMixDataAnnotationsShortSyntax() + public async Task ValidateOnStart_CallValidateOnStartFirst_ValidationSuccessful() { var hostBuilder = CreateHostBuilder(services => { services.AddOptions() + .ValidateOnStart() .Configure(o => { o.StringLength = "111111"; @@ -349,7 +372,6 @@ public async Task CanValidateOnStartMixDataAnnotationsShortSyntax() o.Dep1 = "Not dep2"; }) .ValidateDataAnnotations() - .ValidateOnStart() .Validate(o => o.Custom != "nowhere", "I don't want to go to nowhere!"); }); @@ -370,56 +392,9 @@ public async Task CanValidateOnStartMixDataAnnotationsShortSyntax() } } - [Fact] - public void ValidateOnStart_NullOptionsBuilder_Throws() - { - var hostBuilder = CreateHostBuilder(services => - { - OptionsBuilder optionsBuilder = null; - optionsBuilder.ValidateOnStart(); - }); - - Assert.Throws(() => hostBuilder.Build()); - } - - internal class FakeService { } - - internal class FakeSettings - { - public string Name { get; set; } - } - - [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] - public async Task ValidateOnStart_NamedOptions_FailsOnStartForFailedValidation() - { - var hostBuilder = CreateHostBuilder(services => - { - services.AddOptions().AddSingleton(new FakeService()); - services - .AddOptions("named") - .Configure((o, _) => - { - o.Name = "named"; - }) - .Validate(o => o.Name == null, "trigger validation failure for named option!") - .ValidateOnStart(); - }); - - using (var host = hostBuilder.Build()) - { - var error = await Assert.ThrowsAsync(async () => - { - await host.StartAsync(); - }); - - ValidateFailure(error, 1, "trigger validation failure for named option!"); - } - } - [Fact] [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] - public async Task Test_IVfalidationSuccessful() + public async Task ValidateOnStart_ConfigureBasedOnDataAnnotationRestrictions_ValidationSuccessful() { var hostBuilder = CreateHostBuilder(services => { @@ -463,9 +438,9 @@ private static void ValidateFailure(Type type, OptionsValidationException e, int foreach (var error in errorsToMatch) { #if NETCOREAPP - Assert.True(e.Failures.FirstOrDefault(predicate: f => f.Contains(error, StringComparison.CurrentCulture)) != null, "Did not find: " + error); + Assert.True(e.Failures.FirstOrDefault(predicate: f => f.Contains(error, StringComparison.CurrentCulture)) != null, "Did not find: " + error + " " + e.Failures.First()); #else - Assert.True(e.Failures.FirstOrDefault(predicate: f => f.IndexOf(error, StringComparison.CurrentCulture) >= 0) != null, "Did not find: " + error); + Assert.True(e.Failures.FirstOrDefault(predicate: f => f.IndexOf(error, StringComparison.CurrentCulture) >= 0) != null, "Did not find: " + error + " " + e.Failures.First()); #endif } } From 1c2c3678531fc19ddcee0376254b3030dbeb1e70 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Fri, 5 Feb 2021 12:53:44 -0800 Subject: [PATCH 4/9] Rename --- .../ref/Microsoft.Extensions.Options.cs | 2 +- ...ilderValidationExtensions.cs => OptionsBuilderExtensions.cs} | 2 +- ...ilderValidationTests.cs => OptionsBuilderExtensionsTests.cs} | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) rename src/libraries/Microsoft.Extensions.Options/src/{OptionsBuilderValidationExtensions.cs => OptionsBuilderExtensions.cs} (96%) rename src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/{OptionsBuilderValidationTests.cs => OptionsBuilderExtensionsTests.cs} (99%) diff --git a/src/libraries/Microsoft.Extensions.Options/ref/Microsoft.Extensions.Options.cs b/src/libraries/Microsoft.Extensions.Options/ref/Microsoft.Extensions.Options.cs index 6f0242b508b9c0..818da70a46a9a9 100644 --- a/src/libraries/Microsoft.Extensions.Options/ref/Microsoft.Extensions.Options.cs +++ b/src/libraries/Microsoft.Extensions.Options/ref/Microsoft.Extensions.Options.cs @@ -6,7 +6,7 @@ namespace Microsoft.Extensions.DependencyInjection { - public static partial class OptionsBuilderValidationExtensions + public static partial class OptionsBuilderExtensions { public static Microsoft.Extensions.Options.OptionsBuilder ValidateOnStart(this Microsoft.Extensions.Options.OptionsBuilder optionsBuilder) where TOptions : class { throw null; } } diff --git a/src/libraries/Microsoft.Extensions.Options/src/OptionsBuilderValidationExtensions.cs b/src/libraries/Microsoft.Extensions.Options/src/OptionsBuilderExtensions.cs similarity index 96% rename from src/libraries/Microsoft.Extensions.Options/src/OptionsBuilderValidationExtensions.cs rename to src/libraries/Microsoft.Extensions.Options/src/OptionsBuilderExtensions.cs index a5698a22d4d79b..0d5648e2c1dfb8 100644 --- a/src/libraries/Microsoft.Extensions.Options/src/OptionsBuilderValidationExtensions.cs +++ b/src/libraries/Microsoft.Extensions.Options/src/OptionsBuilderExtensions.cs @@ -9,7 +9,7 @@ namespace Microsoft.Extensions.DependencyInjection /// /// Extension methods for adding configuration related options services to the DI container via . /// - public static class OptionsBuilderValidationExtensions + public static class OptionsBuilderExtensions { /// /// Enforces options validation check on start rather then in runtime. diff --git a/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsBuilderValidationTests.cs b/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsBuilderExtensionsTests.cs similarity index 99% rename from src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsBuilderValidationTests.cs rename to src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsBuilderExtensionsTests.cs index 64aca348fc66e0..6280a2f628909a 100644 --- a/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsBuilderValidationTests.cs +++ b/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsBuilderExtensionsTests.cs @@ -10,7 +10,7 @@ namespace Microsoft.Extensions.Options.Tests { - public class OptionsBuilderValidationTests + public class OptionsBuilderExtensionsTests { public static IHostBuilder CreateHostBuilder(Action configure) { From 8effbfcf63ebec2f52f44205b5965fbfbe2ee4ee Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Tue, 9 Feb 2021 17:49:59 -0800 Subject: [PATCH 5/9] Move to hosting assembly --- .../ref/Microsoft.Extensions.Hosting.cs | 7 +++ .../src/OptionsBuilderExtensions.cs | 0 .../src/ValidationHostedService.cs | 0 .../src/ValidatorOptions.cs | 0 .../tests/UnitTests}/AnnotatedOptions.cs | 0 .../tests/UnitTests}/DepValidatorAttribute.cs | 0 .../tests/UnitTests}/FromAttribute.cs | 0 .../OptionsBuilderExtensionsTests.cs | 0 .../ref/Microsoft.Extensions.Options.cs | 4 -- .../src/Microsoft.Extensions.Options.csproj | 1 - .../Microsoft.Extensions.Options.Tests.csproj | 1 - .../OptionsBuilderTest.cs | 47 +++++++++++++++++++ 12 files changed, 54 insertions(+), 6 deletions(-) rename src/libraries/{Microsoft.Extensions.Options => Microsoft.Extensions.Hosting}/src/OptionsBuilderExtensions.cs (100%) rename src/libraries/{Microsoft.Extensions.Options => Microsoft.Extensions.Hosting}/src/ValidationHostedService.cs (100%) rename src/libraries/{Microsoft.Extensions.Options => Microsoft.Extensions.Hosting}/src/ValidatorOptions.cs (100%) rename src/libraries/{Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests => Microsoft.Extensions.Hosting/tests/UnitTests}/AnnotatedOptions.cs (100%) rename src/libraries/{Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests => Microsoft.Extensions.Hosting/tests/UnitTests}/DepValidatorAttribute.cs (100%) rename src/libraries/{Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests => Microsoft.Extensions.Hosting/tests/UnitTests}/FromAttribute.cs (100%) rename src/libraries/{Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests => Microsoft.Extensions.Hosting/tests/UnitTests}/OptionsBuilderExtensionsTests.cs (100%) diff --git a/src/libraries/Microsoft.Extensions.Hosting/ref/Microsoft.Extensions.Hosting.cs b/src/libraries/Microsoft.Extensions.Hosting/ref/Microsoft.Extensions.Hosting.cs index 5921645e3aca1e..63c5913808ed3b 100644 --- a/src/libraries/Microsoft.Extensions.Hosting/ref/Microsoft.Extensions.Hosting.cs +++ b/src/libraries/Microsoft.Extensions.Hosting/ref/Microsoft.Extensions.Hosting.cs @@ -4,6 +4,13 @@ // Changes to this file must follow the https://aka.ms/api-review process. // ------------------------------------------------------------------------------ +namespace Microsoft.Extensions.DependencyInjection +{ + public static partial class OptionsBuilderExtensions + { + public static Microsoft.Extensions.Options.OptionsBuilder ValidateOnStart(this Microsoft.Extensions.Options.OptionsBuilder optionsBuilder) where TOptions : class { throw null; } + } +} namespace Microsoft.Extensions.Hosting { public partial class ConsoleLifetimeOptions diff --git a/src/libraries/Microsoft.Extensions.Options/src/OptionsBuilderExtensions.cs b/src/libraries/Microsoft.Extensions.Hosting/src/OptionsBuilderExtensions.cs similarity index 100% rename from src/libraries/Microsoft.Extensions.Options/src/OptionsBuilderExtensions.cs rename to src/libraries/Microsoft.Extensions.Hosting/src/OptionsBuilderExtensions.cs diff --git a/src/libraries/Microsoft.Extensions.Options/src/ValidationHostedService.cs b/src/libraries/Microsoft.Extensions.Hosting/src/ValidationHostedService.cs similarity index 100% rename from src/libraries/Microsoft.Extensions.Options/src/ValidationHostedService.cs rename to src/libraries/Microsoft.Extensions.Hosting/src/ValidationHostedService.cs diff --git a/src/libraries/Microsoft.Extensions.Options/src/ValidatorOptions.cs b/src/libraries/Microsoft.Extensions.Hosting/src/ValidatorOptions.cs similarity index 100% rename from src/libraries/Microsoft.Extensions.Options/src/ValidatorOptions.cs rename to src/libraries/Microsoft.Extensions.Hosting/src/ValidatorOptions.cs diff --git a/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/AnnotatedOptions.cs b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/AnnotatedOptions.cs similarity index 100% rename from src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/AnnotatedOptions.cs rename to src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/AnnotatedOptions.cs diff --git a/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/DepValidatorAttribute.cs b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/DepValidatorAttribute.cs similarity index 100% rename from src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/DepValidatorAttribute.cs rename to src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/DepValidatorAttribute.cs diff --git a/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/FromAttribute.cs b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/FromAttribute.cs similarity index 100% rename from src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/FromAttribute.cs rename to src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/FromAttribute.cs diff --git a/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsBuilderExtensionsTests.cs b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/OptionsBuilderExtensionsTests.cs similarity index 100% rename from src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsBuilderExtensionsTests.cs rename to src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/OptionsBuilderExtensionsTests.cs diff --git a/src/libraries/Microsoft.Extensions.Options/ref/Microsoft.Extensions.Options.cs b/src/libraries/Microsoft.Extensions.Options/ref/Microsoft.Extensions.Options.cs index 818da70a46a9a9..d0b8af0aec10d1 100644 --- a/src/libraries/Microsoft.Extensions.Options/ref/Microsoft.Extensions.Options.cs +++ b/src/libraries/Microsoft.Extensions.Options/ref/Microsoft.Extensions.Options.cs @@ -6,10 +6,6 @@ namespace Microsoft.Extensions.DependencyInjection { - public static partial class OptionsBuilderExtensions - { - public static Microsoft.Extensions.Options.OptionsBuilder ValidateOnStart(this Microsoft.Extensions.Options.OptionsBuilder optionsBuilder) where TOptions : class { throw null; } - } public static partial class OptionsServiceCollectionExtensions { public static Microsoft.Extensions.DependencyInjection.IServiceCollection AddOptions(this Microsoft.Extensions.DependencyInjection.IServiceCollection services) { throw null; } diff --git a/src/libraries/Microsoft.Extensions.Options/src/Microsoft.Extensions.Options.csproj b/src/libraries/Microsoft.Extensions.Options/src/Microsoft.Extensions.Options.csproj index bcb9bdd0a34f88..ba7dcec688a67e 100644 --- a/src/libraries/Microsoft.Extensions.Options/src/Microsoft.Extensions.Options.csproj +++ b/src/libraries/Microsoft.Extensions.Options/src/Microsoft.Extensions.Options.csproj @@ -14,7 +14,6 @@ - diff --git a/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/Microsoft.Extensions.Options.Tests.csproj b/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/Microsoft.Extensions.Options.Tests.csproj index f9e8426555e305..e6fb72af73171f 100644 --- a/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/Microsoft.Extensions.Options.Tests.csproj +++ b/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/Microsoft.Extensions.Options.Tests.csproj @@ -8,7 +8,6 @@ - diff --git a/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsBuilderTest.cs b/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsBuilderTest.cs index 86adf09623399f..59514ea56360d4 100644 --- a/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsBuilderTest.cs +++ b/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsBuilderTest.cs @@ -567,6 +567,53 @@ public void CanValidateOptionsEagerly() ValidateFailure(error, Options.DefaultName, 3, "A validation error has occurred.", "Virtual", "Integer"); } + [AttributeUsage(AttributeTargets.Field | AttributeTargets.Property, AllowMultiple = false, Inherited = true)] + public class FromAttribute : ValidationAttribute + { + public string Accepted { get; set; } + + public override bool IsValid(object value) + => value == null || value.ToString() == Accepted; + } + + [AttributeUsage(AttributeTargets.Field | AttributeTargets.Property, AllowMultiple = false, Inherited = true)] + public class DepValidator : ValidationAttribute + { + public string Target { get; set; } + + protected override ValidationResult IsValid(object value, ValidationContext validationContext) + { + object instance = validationContext.ObjectInstance; + Type type = instance.GetType(); + var dep1 = type.GetProperty("Dep1")?.GetValue(instance); + var dep2 = type.GetProperty(Target)?.GetValue(instance); + if (dep1 == dep2) + { + return ValidationResult.Success; + } + return new ValidationResult("Dep1 != "+Target, new string[] { "Dep1", Target }); + } + } + + private class AnnotatedOptions + { + [Required] + public string Required { get; set; } + + [StringLength(5, ErrorMessage = "Too long.")] + public string StringLength { get; set; } + + [Range(-5, 5, ErrorMessage = "Out of range.")] + public int IntRange { get; set; } + + [From(Accepted = "USA")] + public string Custom { get; set; } + + [DepValidator(Target = "Dep2")] + public string Dep1 { get; set; } + public string Dep2 { get; set; } + } + [Fact] public void CanValidateDataAnnotations() { From 9107eab01086c09a5af8daea93d8dce2d45c6c37 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Wed, 10 Feb 2021 10:15:47 -0800 Subject: [PATCH 6/9] - Locate tests properly: Keeps DataAnnotations tests in Options and rest in Hosting --- .../tests/UnitTests/AnnotatedOptions.cs | 27 ---- .../tests/UnitTests/ComplexOptions.cs | 50 ++++++ .../tests/UnitTests/DepValidatorAttribute.cs | 29 ---- .../tests/UnitTests/FromAttribute.cs | 17 -- .../OptionsBuilderExtensionsTests.cs | 146 +----------------- .../Microsoft.Extensions.Options.Tests.csproj | 1 + .../OptionsBuilderTest.cs | 124 ++++++++++++++- 7 files changed, 175 insertions(+), 219 deletions(-) delete mode 100644 src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/AnnotatedOptions.cs create mode 100644 src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/ComplexOptions.cs delete mode 100644 src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/DepValidatorAttribute.cs delete mode 100644 src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/FromAttribute.cs diff --git a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/AnnotatedOptions.cs b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/AnnotatedOptions.cs deleted file mode 100644 index 4225282168c4bc..00000000000000 --- a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/AnnotatedOptions.cs +++ /dev/null @@ -1,27 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.ComponentModel.DataAnnotations; -using System; - -namespace Microsoft.Extensions.Options.Tests -{ - public class AnnotatedOptions - { - [Required] - public string Required { get; set; } - - [StringLength(5, ErrorMessage = "Too long.")] - public string StringLength { get; set; } - - [Range(-5, 5, ErrorMessage = "Out of range.")] - public int IntRange { get; set; } - - [From(Accepted = "USA")] - public string Custom { get; set; } - - [DepValidator(Target = "Dep2")] - public string Dep1 { get; set; } - public string Dep2 { get; set; } - } -} diff --git a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/ComplexOptions.cs b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/ComplexOptions.cs new file mode 100644 index 00000000000000..81df525e11d0d9 --- /dev/null +++ b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/ComplexOptions.cs @@ -0,0 +1,50 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; + +namespace Microsoft.Extensions.Hosting.Tests +{ + public class ComplexOptions + { + public ComplexOptions() + { + Nested = new NestedOptions(); + Virtual = "complex"; + } + public NestedOptions Nested { get; set; } + public int Integer { get; set; } + public bool Boolean { get; set; } + public virtual string Virtual { get; set; } + + public string PrivateSetter { get; private set; } + public string ProtectedSetter { get; protected set; } + public string InternalSetter { get; internal set; } + public static string StaticProperty { get; set; } + + public string ReadOnly + { + get { return null; } + } + } + + public class NestedOptions + { + public int Integer { get; set; } + } + + public class DerivedOptions : ComplexOptions + { + public override string Virtual + { + get + { + return base.Virtual; + } + set + { + base.Virtual = "Derived:" + value; + } + } + } +} diff --git a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/DepValidatorAttribute.cs b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/DepValidatorAttribute.cs deleted file mode 100644 index 330e16f13a033d..00000000000000 --- a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/DepValidatorAttribute.cs +++ /dev/null @@ -1,29 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.ComponentModel.DataAnnotations; -using System; - -namespace Microsoft.Extensions.Options.Tests -{ - [AttributeUsage(AttributeTargets.Field | AttributeTargets.Property, AllowMultiple = false, Inherited = true)] - public sealed class DepValidatorAttribute - : ValidationAttribute - { - public string Target { get; set; } - - protected override ValidationResult IsValid(object value, ValidationContext validationContext) - { - object instance = validationContext.ObjectInstance; - Type type = instance.GetType(); - var dep1 = type.GetProperty("Dep1")?.GetValue(instance); - var dep2 = type.GetProperty(Target)?.GetValue(instance); - if (dep1 == dep2) - { - return ValidationResult.Success; - } - - return new ValidationResult("Dep1 != " + Target, new[] { "Dep1", Target }); - } - } -} diff --git a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/FromAttribute.cs b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/FromAttribute.cs deleted file mode 100644 index f16e7f0726baac..00000000000000 --- a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/FromAttribute.cs +++ /dev/null @@ -1,17 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.ComponentModel.DataAnnotations; -using System; - -namespace Microsoft.Extensions.Options.Tests -{ - [AttributeUsage(AttributeTargets.Field | AttributeTargets.Property, AllowMultiple = false, Inherited = true)] - public sealed class FromAttribute : ValidationAttribute - { - public string Accepted { get; set; } - - public override bool IsValid(object value) - => value == null || value.ToString() == Accepted; - } -} diff --git a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/OptionsBuilderExtensionsTests.cs b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/OptionsBuilderExtensionsTests.cs index 6280a2f628909a..b47ed88c5648e5 100644 --- a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/OptionsBuilderExtensionsTests.cs +++ b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/OptionsBuilderExtensionsTests.cs @@ -8,7 +8,7 @@ using Microsoft.Extensions.Hosting; using Xunit; -namespace Microsoft.Extensions.Options.Tests +namespace Microsoft.Extensions.Hosting.Tests { public class OptionsBuilderExtensionsTests { @@ -22,7 +22,7 @@ public void ValidateOnStart_NullOptionsBuilder_Throws() { var hostBuilder = CreateHostBuilder(services => { - OptionsBuilder optionsBuilder = null; + OptionsBuilder optionsBuilder = null; optionsBuilder.ValidateOnStart(); }); @@ -286,148 +286,6 @@ public async Task ValidateOnStart_MultipleErrorsInOneValidationCallUsingCustomEr } } - [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] - public async Task ValidateOnStart_CallValidateDataAnnotations_ValidationSuccessful() - { - var hostBuilder = CreateHostBuilder(services => - { - services.AddOptions() - .Configure(o => - { - o.StringLength = "111111"; - o.IntRange = 10; - o.Custom = "nowhere"; - o.Dep1 = "Not dep2"; - }) - .ValidateDataAnnotations() - .ValidateOnStart(); - }); - - using (var host = hostBuilder.Build()) - { - var error = await Assert.ThrowsAsync(async () => - { - await host.StartAsync(); - }); - - ValidateFailure(error, 5, - "DataAnnotation validation failed for members: 'Required' with the error: 'The Required field is required.'.", - "DataAnnotation validation failed for members: 'StringLength' with the error: 'Too long.'.", - "DataAnnotation validation failed for members: 'IntRange' with the error: 'Out of range.'.", - "DataAnnotation validation failed for members: 'Custom' with the error: 'The field Custom is invalid.'.", - "DataAnnotation validation failed for members: 'Dep1,Dep2' with the error: 'Dep1 != Dep2'."); - } - } - - [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] - public async Task ValidateOnStart_CallValidateAndValidateDataAnnotations_FailuresCaughtFromBothValidateAndValidateDataAnnotations() - { - var hostBuilder = CreateHostBuilder(services => - { - services.AddOptions() - .Configure(o => - { - o.StringLength = "111111"; - o.IntRange = 10; - o.Custom = "nowhere"; - o.Dep1 = "Not dep2"; - }) - .ValidateDataAnnotations() - .Validate(o => o.Custom != "nowhere", "I don't want to go to nowhere!") - .ValidateOnStart(); - }); - - using (var host = hostBuilder.Build()) - { - var error = await Assert.ThrowsAsync(async () => - { - await host.StartAsync(); - }); - - ValidateFailure(error, 6, - "DataAnnotation validation failed for members: 'Required' with the error: 'The Required field is required.'.", - "DataAnnotation validation failed for members: 'StringLength' with the error: 'Too long.'.", - "DataAnnotation validation failed for members: 'IntRange' with the error: 'Out of range.'.", - "DataAnnotation validation failed for members: 'Custom' with the error: 'The field Custom is invalid.'.", - "DataAnnotation validation failed for members: 'Dep1,Dep2' with the error: 'Dep1 != Dep2'.", - "I don't want to go to nowhere!"); - } - } - - [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] - public async Task ValidateOnStart_CallValidateOnStartFirst_ValidationSuccessful() - { - var hostBuilder = CreateHostBuilder(services => - { - services.AddOptions() - .ValidateOnStart() - .Configure(o => - { - o.StringLength = "111111"; - o.IntRange = 10; - o.Custom = "nowhere"; - o.Dep1 = "Not dep2"; - }) - .ValidateDataAnnotations() - .Validate(o => o.Custom != "nowhere", "I don't want to go to nowhere!"); - }); - - using (var host = hostBuilder.Build()) - { - var error = await Assert.ThrowsAsync(async () => - { - await host.StartAsync(); - }); - - ValidateFailure(error, 6, - "DataAnnotation validation failed for members: 'Required' with the error: 'The Required field is required.'.", - "DataAnnotation validation failed for members: 'StringLength' with the error: 'Too long.'.", - "DataAnnotation validation failed for members: 'IntRange' with the error: 'Out of range.'.", - "DataAnnotation validation failed for members: 'Custom' with the error: 'The field Custom is invalid.'.", - "DataAnnotation validation failed for members: 'Dep1,Dep2' with the error: 'Dep1 != Dep2'.", - "I don't want to go to nowhere!"); - } - } - - [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] - public async Task ValidateOnStart_ConfigureBasedOnDataAnnotationRestrictions_ValidationSuccessful() - { - var hostBuilder = CreateHostBuilder(services => - { - services.AddOptions() - .Configure(o => - { - o.Required = "required"; - o.StringLength = "1111"; - o.IntRange = 0; - o.Custom = "USA"; - o.Dep1 = "dep"; - o.Dep2 = "dep"; - }) - .ValidateDataAnnotations() - .ValidateOnStart() - .Validate(o => o.Custom != "nowhere", "I don't want to go to nowhere!"); - }); - - using (var host = hostBuilder.Build()) - { - try - { - await host.StartAsync(); - } - #pragma warning disable CA1031 // Do not catch general exception types - catch (Exception ex) - #pragma warning restore CA1031 // Do not catch general exception types - { - Assert.True(false, "Expected no exception, but got: " + ex.Message); - } - } - } - private static void ValidateFailure(Type type, OptionsValidationException e, int count = 1, params string[] errorsToMatch) { Assert.Equal(type, e.OptionsType); diff --git a/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/Microsoft.Extensions.Options.Tests.csproj b/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/Microsoft.Extensions.Options.Tests.csproj index e6fb72af73171f..f9e8426555e305 100644 --- a/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/Microsoft.Extensions.Options.Tests.csproj +++ b/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/Microsoft.Extensions.Options.Tests.csproj @@ -8,6 +8,7 @@ + diff --git a/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsBuilderTest.cs b/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsBuilderTest.cs index 59514ea56360d4..67d063625ac5f3 100644 --- a/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsBuilderTest.cs +++ b/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsBuilderTest.cs @@ -399,7 +399,7 @@ private void ValidateFailure(OptionsValidationException e, string name // Check for the error in any of the failures foreach (var error in errorsToMatch) { - Assert.True(e.Failures.FirstOrDefault(f => f.Contains(error)) != null, "Did not find: "+error); + Assert.True(e.Failures.FirstOrDefault(f => f.Contains(error)) != null, "Did not find: " + error); } Assert.Equal(e.Message, String.Join("; ", e.Failures)); } @@ -591,7 +591,7 @@ protected override ValidationResult IsValid(object value, ValidationContext vali { return ValidationResult.Success; } - return new ValidationResult("Dep1 != "+Target, new string[] { "Dep1", Target }); + return new ValidationResult("Dep1 != " + Target, new string[] { "Dep1", Target }); } } @@ -665,5 +665,125 @@ public void CanValidateMixDataAnnotations() "DataAnnotation validation failed for members: 'Dep1,Dep2' with the error: 'Dep1 != Dep2'.", "I don't want to go to nowhere!"); } + + [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] + public void ValidateOnStart_CallValidateDataAnnotations_ValidationSuccessful() + { + var services = new ServiceCollection(); + services.AddOptions() + .Configure(o => + { + o.StringLength = "111111"; + o.IntRange = 10; + o.Custom = "nowhere"; + o.Dep1 = "Not dep2"; + }) + .ValidateDataAnnotations() + .ValidateOnStart(); + + var sp = services.BuildServiceProvider(); + + var error = Assert.Throws(() => sp.GetRequiredService>().Value); + ValidateFailure(error, Options.DefaultName, 5, + "DataAnnotation validation failed for members: 'Required' with the error: 'The Required field is required.'.", + "DataAnnotation validation failed for members: 'StringLength' with the error: 'Too long.'.", + "DataAnnotation validation failed for members: 'IntRange' with the error: 'Out of range.'.", + "DataAnnotation validation failed for members: 'Custom' with the error: 'The field Custom is invalid.'.", + "DataAnnotation validation failed for members: 'Dep1,Dep2' with the error: 'Dep1 != Dep2'."); + } + + [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] + public void ValidateOnStart_CallValidateAndValidateDataAnnotations_FailuresCaughtFromBothValidateAndValidateDataAnnotations() + { + var services = new ServiceCollection(); + + services.AddOptions() + .Configure(o => + { + o.StringLength = "111111"; + o.IntRange = 10; + o.Custom = "nowhere"; + o.Dep1 = "Not dep2"; + }) + .ValidateDataAnnotations() + .Validate(o => o.Custom != "nowhere", "I don't want to go to nowhere!") + .ValidateOnStart(); + + var sp = services.BuildServiceProvider(); + + var error = Assert.Throws(() => sp.GetRequiredService>().Value); + ValidateFailure(error, Options.DefaultName, 6, + "DataAnnotation validation failed for members: 'Required' with the error: 'The Required field is required.'.", + "DataAnnotation validation failed for members: 'StringLength' with the error: 'Too long.'.", + "DataAnnotation validation failed for members: 'IntRange' with the error: 'Out of range.'.", + "DataAnnotation validation failed for members: 'Custom' with the error: 'The field Custom is invalid.'.", + "DataAnnotation validation failed for members: 'Dep1,Dep2' with the error: 'Dep1 != Dep2'.", + "I don't want to go to nowhere!"); + } + + [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] + public void ValidateOnStart_CallValidateOnStartFirst_ValidationSuccessful() + { + var services = new ServiceCollection(); + + services.AddOptions() + .ValidateOnStart() + .Configure(o => + { + o.StringLength = "111111"; + o.IntRange = 10; + o.Custom = "nowhere"; + o.Dep1 = "Not dep2"; + }) + .ValidateDataAnnotations() + .Validate(o => o.Custom != "nowhere", "I don't want to go to nowhere!"); + + var sp = services.BuildServiceProvider(); + + var error = Assert.Throws(() => sp.GetRequiredService>().Value); + ValidateFailure(error, Options.DefaultName, 6, + "DataAnnotation validation failed for members: 'Required' with the error: 'The Required field is required.'.", + "DataAnnotation validation failed for members: 'StringLength' with the error: 'Too long.'.", + "DataAnnotation validation failed for members: 'IntRange' with the error: 'Out of range.'.", + "DataAnnotation validation failed for members: 'Custom' with the error: 'The field Custom is invalid.'.", + "DataAnnotation validation failed for members: 'Dep1,Dep2' with the error: 'Dep1 != Dep2'.", + "I don't want to go to nowhere!"); + } + + [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] + public void ValidateOnStart_ConfigureBasedOnDataAnnotationRestrictions_ValidationSuccessful() + { + var services = new ServiceCollection(); + services.AddOptions() + .Configure(o => + { + o.Required = "required"; + o.StringLength = "1111"; + o.IntRange = 0; + o.Custom = "USA"; + o.Dep1 = "dep"; + o.Dep2 = "dep"; + }) + .ValidateDataAnnotations() + .ValidateOnStart() + .Validate(o => o.Custom != "nowhere", "I don't want to go to nowhere!"); + + var sp = services.BuildServiceProvider(); + + try + { + var value = sp.GetRequiredService>().Value; + } +#pragma warning disable CA1031 // Do not catch general exception types + catch (Exception ex) +#pragma warning restore CA1031 // Do not catch general exception types + { + Assert.True(false, "Expected no exception, but got: " + ex.Message); + } + } } } From 1f36ad3430d1bd0ff980300502ffdd9fede3650c Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Wed, 10 Feb 2021 11:37:32 -0800 Subject: [PATCH 7/9] fix compile --- .../tests/UnitTests/OptionsBuilderExtensionsTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/OptionsBuilderExtensionsTests.cs b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/OptionsBuilderExtensionsTests.cs index b47ed88c5648e5..5dc50b7e4d2a27 100644 --- a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/OptionsBuilderExtensionsTests.cs +++ b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/OptionsBuilderExtensionsTests.cs @@ -6,6 +6,7 @@ using System.Threading.Tasks; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Options; using Xunit; namespace Microsoft.Extensions.Hosting.Tests From 6594f84763ebcec9e5aced6a17218aec52340872 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Fri, 12 Feb 2021 13:47:51 -0800 Subject: [PATCH 8/9] Apply PR feedback. Improve unit tests --- .../src/OptionsBuilderExtensions.cs | 2 +- .../OptionsBuilderExtensionsTests.cs | 103 +++++++++++++----- .../OptionsBuilderTest.cs | 15 +-- 3 files changed, 80 insertions(+), 40 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Hosting/src/OptionsBuilderExtensions.cs b/src/libraries/Microsoft.Extensions.Hosting/src/OptionsBuilderExtensions.cs index 0d5648e2c1dfb8..6ad4d22ae90c73 100644 --- a/src/libraries/Microsoft.Extensions.Hosting/src/OptionsBuilderExtensions.cs +++ b/src/libraries/Microsoft.Extensions.Hosting/src/OptionsBuilderExtensions.cs @@ -12,7 +12,7 @@ namespace Microsoft.Extensions.DependencyInjection public static class OptionsBuilderExtensions { /// - /// Enforces options validation check on start rather then in runtime. + /// Enforces options validation check on start rather than in runtime. /// /// The type of options. /// The to configure options instance. diff --git a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/OptionsBuilderExtensionsTests.cs b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/OptionsBuilderExtensionsTests.cs index 5dc50b7e4d2a27..1a57d74b54403e 100644 --- a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/OptionsBuilderExtensionsTests.cs +++ b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/OptionsBuilderExtensionsTests.cs @@ -21,13 +21,7 @@ public static IHostBuilder CreateHostBuilder(Action configur [Fact] public void ValidateOnStart_NullOptionsBuilder_Throws() { - var hostBuilder = CreateHostBuilder(services => - { - OptionsBuilder optionsBuilder = null; - optionsBuilder.ValidateOnStart(); - }); - - Assert.Throws(() => hostBuilder.Build()); + Assert.Throws(() => OptionsBuilderExtensions.ValidateOnStart(null)); } [Fact] @@ -136,13 +130,19 @@ public async Task ValidateOnStart_NamedOptions_ValidatesFailureOnStart() [Fact] [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] - private async Task ValidateOnStart_AddOptionsMultipleTimesForSameType_LastValidateOnStartCallGetsTriggered() + private async Task ValidateOnStart_AddOptionsMultipleTimesForSameType_LastOneGetsTriggered() { + bool firstOptionsBuilderIgnored = true; + bool secondOptionsBuilderTriggered = false; var hostBuilder = CreateHostBuilder(services => { services.AddOptions("bad_configuration1") .Configure(o => o.Boolean = false) - .Validate(o => o.Boolean, "bad_configuration1") + .Validate(o => + { + firstOptionsBuilderIgnored = false; + return o.Boolean; + }, "bad_configuration1") .ValidateOnStart(); services.AddOptions("bad_configuration2") @@ -151,7 +151,11 @@ private async Task ValidateOnStart_AddOptionsMultipleTimesForSameType_LastValida o.Boolean = false; o.Integer = 11; }) - .Validate(o => o.Boolean, "Boolean") + .Validate(o => + { + secondOptionsBuilderTriggered = true; + return o.Boolean; + }, "Boolean") .Validate(o => o.Integer > 12, "Integer") .ValidateOnStart(); }); @@ -165,12 +169,44 @@ private async Task ValidateOnStart_AddOptionsMultipleTimesForSameType_LastValida ValidateFailure(error, 2, "Boolean", "Integer"); } + + Assert.True(firstOptionsBuilderIgnored); + Assert.True(secondOptionsBuilderTriggered); + } + + [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] + private async Task ValidateOnStart_AddEagerValidation_DoesValidationWhenHostStartsWithNoFailure() + { + bool validateCalled = false; + + var hostBuilder = CreateHostBuilder(services => + { + // Adds eager validation using ValidateOnStart + services.AddOptions("correct_configuration") + .Configure(o => o.Boolean = true) + .Validate(o => + { + validateCalled = true; + return o.Boolean; + }, "correct_configuration") + .ValidateOnStart(); + }); + + using (var host = hostBuilder.Build()) + { + await host.StartAsync(); + } + + Assert.True(validateCalled); } [Fact] [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] - private async Task ValidateOnStart_AddBothLazyAndEagerValidationOnSameType_SkipsLazyValidationWhenHostStarts() + private async Task ValidateOnStart_AddLazyValidation_SkipsValidationWhenHostStarts() { + bool validateCalled = false; + var hostBuilder = CreateHostBuilder(services => { // Adds eager validation using ValidateOnStart @@ -179,43 +215,51 @@ private async Task ValidateOnStart_AddBothLazyAndEagerValidationOnSameType_Skips .Validate(o => o.Boolean, "correct_configuration") .ValidateOnStart(); - // Adds lazy validation, skipping validation on start + // Adds lazy validation, skipping validation on start (last options builder for same type gets triggered so above one is skipped) services.AddOptions("bad_configuration") .Configure(o => o.Boolean = false) - .Validate(o => o.Boolean, "bad_configuration"); + .Validate(o => + { + validateCalled = true; + return o.Boolean; + }, "bad_configuration"); }); // For the lazily added "bad_configuration", validation failure does not occur when host starts using (var host = hostBuilder.Build()) { - try - { - await host.StartAsync(); - } - #pragma warning disable CA1031 // Do not catch general exception types - catch (Exception ex) - #pragma warning restore CA1031 // Do not catch general exception types - { - Assert.True(false, "Expected no exception, but got: " + ex.Message); - } + await host.StartAsync(); } + + Assert.False(validateCalled); } [Fact] [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] - public async Task ValidateOnStart_AddBothLazyAndEagerValidationOnDifferentTypes_SkipsLazyValidationWhenHostStarts() + public async Task ValidateOnStart_AddBothLazyAndEagerValidationOnDifferentTypes_ValidatesWhenHostStartsOnlyForEagerValidations() { + bool validateCalledForNested = false; + bool validateCalledForComplexOptions = false; + var hostBuilder = CreateHostBuilder(services => { - // Lazy validation + // Lazy validation for NestedOptions services.AddOptions() .Configure(o => o.Integer = 11) - .Validate(o => o.Integer > 12, "Integer"); + .Validate(o => + { + validateCalledForNested = true; + return o.Integer > 12; + }, "Integer"); - // Eager validation + // Eager validation for ComplexOptions services.AddOptions() .Configure(o => o.Boolean = false) - .Validate(o => o.Boolean, "first Boolean must be true.") + .Validate(o => + { + validateCalledForComplexOptions = true; + return o.Boolean; + }, "first Boolean must be true.") .ValidateOnStart(); }); @@ -228,6 +272,9 @@ public async Task ValidateOnStart_AddBothLazyAndEagerValidationOnDifferentTypes_ ValidateFailure(error, 1, "first Boolean must be true."); } + + Assert.False(validateCalledForNested); + Assert.True(validateCalledForComplexOptions); } [Fact] diff --git a/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsBuilderTest.cs b/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsBuilderTest.cs index 67d063625ac5f3..9e44a897d2332b 100644 --- a/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsBuilderTest.cs +++ b/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsBuilderTest.cs @@ -725,7 +725,7 @@ public void ValidateOnStart_CallValidateAndValidateDataAnnotations_FailuresCaugh [Fact] [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] - public void ValidateOnStart_CallValidateOnStartFirst_ValidationSuccessful() + public void ValidateOnStart_CallValidateOnStartFirst_ValidatesFailuresCorrectly() { var services = new ServiceCollection(); @@ -774,16 +774,9 @@ public void ValidateOnStart_ConfigureBasedOnDataAnnotationRestrictions_Validatio var sp = services.BuildServiceProvider(); - try - { - var value = sp.GetRequiredService>().Value; - } -#pragma warning disable CA1031 // Do not catch general exception types - catch (Exception ex) -#pragma warning restore CA1031 // Do not catch general exception types - { - Assert.True(false, "Expected no exception, but got: " + ex.Message); - } + var value = sp.GetRequiredService>().Value; + + Assert.NotNull(value); } } } From 23b6ea3c2d78c3b3a0477c5a60da0dae60455565 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Tue, 16 Feb 2021 16:43:33 -0800 Subject: [PATCH 9/9] Apply nit feedback --- .../tests/UnitTests/OptionsBuilderExtensionsTests.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/OptionsBuilderExtensionsTests.cs b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/OptionsBuilderExtensionsTests.cs index 1a57d74b54403e..e8aaff44181e29 100644 --- a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/OptionsBuilderExtensionsTests.cs +++ b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/OptionsBuilderExtensionsTests.cs @@ -132,7 +132,7 @@ public async Task ValidateOnStart_NamedOptions_ValidatesFailureOnStart() [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] private async Task ValidateOnStart_AddOptionsMultipleTimesForSameType_LastOneGetsTriggered() { - bool firstOptionsBuilderIgnored = true; + bool firstOptionsBuilderTriggered = false; bool secondOptionsBuilderTriggered = false; var hostBuilder = CreateHostBuilder(services => { @@ -140,7 +140,7 @@ private async Task ValidateOnStart_AddOptionsMultipleTimesForSameType_LastOneGet .Configure(o => o.Boolean = false) .Validate(o => { - firstOptionsBuilderIgnored = false; + firstOptionsBuilderTriggered = true; return o.Boolean; }, "bad_configuration1") .ValidateOnStart(); @@ -170,7 +170,7 @@ private async Task ValidateOnStart_AddOptionsMultipleTimesForSameType_LastOneGet ValidateFailure(error, 2, "Boolean", "Integer"); } - Assert.True(firstOptionsBuilderIgnored); + Assert.False(firstOptionsBuilderTriggered); Assert.True(secondOptionsBuilderTriggered); }