Skip to content

Commit

Permalink
keep it simple: don't try to resolve the DataSource from the DI if on…
Browse files Browse the repository at this point in the history
…ly a connection string was provided
  • Loading branch information
adamsitnik committed Dec 15, 2023
1 parent 07ae4b5 commit 3e8a1be
Show file tree
Hide file tree
Showing 3 changed files with 1 addition and 68 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,6 @@ public static IHealthChecksBuilder AddNpgSql(
{
options.ConnectionString ??= Guard.ThrowIfNull(connectionStringFactory.Invoke(sp), throwOnEmptyString: true, paramName: nameof(connectionStringFactory));
ResolveDataSourceIfPossible(options, sp);
return new NpgSqlHealthCheck(options);
},
failureStatus,
Expand Down Expand Up @@ -170,29 +168,9 @@ public static IHealthChecksBuilder AddNpgSql(

return builder.Add(new HealthCheckRegistration(
name ?? NAME,
sp =>
{
ResolveDataSourceIfPossible(options, sp);
return new NpgSqlHealthCheck(options);
},
_ => new NpgSqlHealthCheck(options),
failureStatus,
tags,
timeout));
}

private static void ResolveDataSourceIfPossible(NpgSqlHealthCheckOptions options, IServiceProvider sp)
{
if (options.DataSource is null && !options.TriedToResolveFromDI)
{
NpgsqlDataSource? fromDi = sp.GetService<NpgsqlDataSource>();
if (fromDi?.ConnectionString == options.ConnectionString)
{
// When it's possible, we reuse the DataSource registered in the DI.
// We do that to achieve best performance and avoid issues like https://github.com/Xabaril/AspNetCore.Diagnostics.HealthChecks/issues/1993
options.DataSource = fromDi;
}
options.TriedToResolveFromDI = true; // save the answer, so we don't do it more than once
}
}
}
2 changes: 0 additions & 2 deletions src/HealthChecks.NpgSql/NpgSqlHealthCheckOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,6 @@ public NpgSqlHealthCheckOptions(NpgsqlDataSource dataSource)
/// </remarks>
public NpgsqlDataSource? DataSource { get; internal set; }

internal bool TriedToResolveFromDI;

/// <summary>
/// The query to be executed.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
using System.Reflection;
using HealthChecks.NpgSql;
using Npgsql;

namespace HealthChecks.Npgsql.Tests.DependencyInjection;

Expand All @@ -21,7 +19,6 @@ public void add_health_check_when_properly_configured()

registration.Name.ShouldBe("npgsql");
check.ShouldBeOfType<NpgSqlHealthCheck>();

}

[Fact]
Expand Down Expand Up @@ -91,46 +88,6 @@ public void factory_is_called_only_once()
factoryCalls.ShouldBe(1);
}

[Theory]
[InlineData(true)]
[InlineData(false)]
public void factory_reuses_pre_registered_datasource_when_possible(bool sameConnectionString)
{
const string connectionString = "Host=localhost";
ServiceCollection services = new();

services.AddSingleton<NpgsqlDataSource>(serviceProvider =>
{
var dataSourceBuilder = new NpgsqlDataSourceBuilder(connectionString);
return dataSourceBuilder.Build();
});

int factoryCalls = 0;
services.AddHealthChecks()
.AddNpgSql(_ =>
{
Interlocked.Increment(ref factoryCalls);
return sameConnectionString ? connectionString : $"{connectionString}2";
}, name: "my-npg-1");

using var serviceProvider = services.BuildServiceProvider();

var options = serviceProvider.GetRequiredService<IOptions<HealthCheckServiceOptions>>();

var registration = options.Value.Registrations.Single();

for (int i = 0; i < 10; i++)
{
var healthCheck = (NpgSqlHealthCheck)registration.Factory(serviceProvider);
var fieldInfo = typeof(NpgSqlHealthCheck).GetField("_options", BindingFlags.Instance | BindingFlags.NonPublic);
var npgSqlHealthCheckOptions = (NpgSqlHealthCheckOptions)fieldInfo!.GetValue(healthCheck)!;

Assert.Equal(sameConnectionString, ReferenceEquals(serviceProvider.GetRequiredService<NpgsqlDataSource>(), npgSqlHealthCheckOptions.DataSource));
}

factoryCalls.ShouldBe(1);
}

[Fact]
public void recommended_scenario_compiles_and_works_as_expected()
{
Expand Down

0 comments on commit 3e8a1be

Please sign in to comment.