From c1b285d551a178de4e9538e88b6f5aaedc4777a4 Mon Sep 17 00:00:00 2001 From: Wei Chen Date: Sat, 3 Oct 2020 17:22:38 +1300 Subject: [PATCH] Move validation of HealthCheckServiceOptions --- .../src/DefaultHealthCheckService.cs | 26 ++++--------------- .../HealthCheckServiceCollectionExtensions.cs | 3 +++ .../HealthCheckServiceOptionsValidation.cs | 26 +++++++++++++++++++ .../test/DefaultHealthCheckServiceTest.cs | 6 +---- 4 files changed, 35 insertions(+), 26 deletions(-) create mode 100644 src/HealthChecks/HealthChecks/src/DependencyInjection/HealthCheckServiceOptionsValidation.cs diff --git a/src/HealthChecks/HealthChecks/src/DefaultHealthCheckService.cs b/src/HealthChecks/HealthChecks/src/DefaultHealthCheckService.cs index 96e62660c9d1..a9deac7d8523 100644 --- a/src/HealthChecks/HealthChecks/src/DefaultHealthCheckService.cs +++ b/src/HealthChecks/HealthChecks/src/DefaultHealthCheckService.cs @@ -18,7 +18,7 @@ namespace Microsoft.Extensions.Diagnostics.HealthChecks internal class DefaultHealthCheckService : HealthCheckService { private readonly IServiceScopeFactory _scopeFactory; - private readonly IOptions _options; + private readonly HealthCheckServiceOptions _options; private readonly ILogger _logger; public DefaultHealthCheckService( @@ -27,19 +27,18 @@ public DefaultHealthCheckService( ILogger logger) { _scopeFactory = scopeFactory ?? throw new ArgumentNullException(nameof(scopeFactory)); - _options = options ?? throw new ArgumentNullException(nameof(options)); - _logger = logger ?? throw new ArgumentNullException(nameof(logger)); - // We're specifically going out of our way to do this at startup time. We want to make sure you // get any kind of health-check related error as early as possible. Waiting until someone // actually tries to **run** health checks would be real baaaaad. - ValidateRegistrations(_options.Value.Registrations); + _options = options?.Value ?? throw new ArgumentNullException(nameof(options)); + _logger = logger ?? throw new ArgumentNullException(nameof(logger)); } + public override async Task CheckHealthAsync( Func? predicate, CancellationToken cancellationToken = default) { - var registrations = _options.Value.Registrations; + var registrations = _options.Registrations; if (predicate != null) { registrations = registrations.Where(predicate).ToArray(); @@ -155,21 +154,6 @@ private async Task RunCheckAsync(IServiceScope scope, HealthC } } - private static void ValidateRegistrations(IEnumerable registrations) - { - // Scan the list for duplicate names to provide a better error if there are duplicates. - var duplicateNames = registrations - .GroupBy(c => c.Name, StringComparer.OrdinalIgnoreCase) - .Where(g => g.Count() > 1) - .Select(g => g.Key) - .ToList(); - - if (duplicateNames.Count > 0) - { - throw new ArgumentException($"Duplicate health checks were registered with the name(s): {string.Join(", ", duplicateNames)}", nameof(registrations)); - } - } - internal static class EventIds { public static readonly EventId HealthCheckProcessingBegin = new EventId(100, "HealthCheckProcessingBegin"); diff --git a/src/HealthChecks/HealthChecks/src/DependencyInjection/HealthCheckServiceCollectionExtensions.cs b/src/HealthChecks/HealthChecks/src/DependencyInjection/HealthCheckServiceCollectionExtensions.cs index 91ffa59449e3..d375071dbfca 100644 --- a/src/HealthChecks/HealthChecks/src/DependencyInjection/HealthCheckServiceCollectionExtensions.cs +++ b/src/HealthChecks/HealthChecks/src/DependencyInjection/HealthCheckServiceCollectionExtensions.cs @@ -3,7 +3,9 @@ using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.Diagnostics.HealthChecks; +using Microsoft.Extensions.Diagnostics.HealthChecks.DependencyInjection; using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Options; namespace Microsoft.Extensions.DependencyInjection { @@ -25,6 +27,7 @@ public static class HealthCheckServiceCollectionExtensions /// An instance of from which health checks can be registered. public static IHealthChecksBuilder AddHealthChecks(this IServiceCollection services) { + services.TryAddEnumerable(ServiceDescriptor.Transient, HealthCheckServiceOptionsValidation>()); services.TryAddSingleton(); services.TryAddEnumerable(ServiceDescriptor.Singleton()); return new HealthChecksBuilder(services); diff --git a/src/HealthChecks/HealthChecks/src/DependencyInjection/HealthCheckServiceOptionsValidation.cs b/src/HealthChecks/HealthChecks/src/DependencyInjection/HealthCheckServiceOptionsValidation.cs new file mode 100644 index 000000000000..7cbd8cae649c --- /dev/null +++ b/src/HealthChecks/HealthChecks/src/DependencyInjection/HealthCheckServiceOptionsValidation.cs @@ -0,0 +1,26 @@ +using System; +using System.Linq; +using Microsoft.Extensions.Options; + +namespace Microsoft.Extensions.Diagnostics.HealthChecks.DependencyInjection +{ + internal class HealthCheckServiceOptionsValidation : IValidateOptions + { + public ValidateOptionsResult Validate(string name, HealthCheckServiceOptions options) + { + // Scan the list for duplicate names to provide a better error if there are duplicates. + var duplicateNames = options.Registrations + .GroupBy(c => c.Name, StringComparer.OrdinalIgnoreCase) + .Where(g => g.Count() > 1) + .Select(g => g.Key) + .ToList(); + + if (duplicateNames.Count > 0) + { + return ValidateOptionsResult.Fail($"Duplicate health checks were registered with the name(s): {string.Join(", ", duplicateNames)}"); + } + + return ValidateOptionsResult.Success; + } + } +} diff --git a/src/HealthChecks/HealthChecks/test/DefaultHealthCheckServiceTest.cs b/src/HealthChecks/HealthChecks/test/DefaultHealthCheckServiceTest.cs index 430516bc0463..358b63f2b700 100644 --- a/src/HealthChecks/HealthChecks/test/DefaultHealthCheckServiceTest.cs +++ b/src/HealthChecks/HealthChecks/test/DefaultHealthCheckServiceTest.cs @@ -39,12 +39,8 @@ public void Constructor_ThrowsUsefulExceptionForDuplicateNames() var services = serviceCollection.BuildServiceProvider(); - var scopeFactory = services.GetRequiredService(); - var options = services.GetRequiredService>(); - var logger = services.GetRequiredService>(); - // Act - var exception = Assert.Throws(() => new DefaultHealthCheckService(scopeFactory, options, logger)); + var exception = Assert.Throws(() => services.GetRequiredService()); // Assert Assert.StartsWith($"Duplicate health checks were registered with the name(s): Foo, Baz", exception.Message);