TCP R1-R4 + C9: declare and enforce the public API surface

ClickHouse.Driver.Common/.editorconfig

@@ -0,0 +1,5 @@
+[*.cs]
+dotnet_diagnostic.RS0016.severity = error # RS0016: Public APIs must be declared
+dotnet_diagnostic.RS0017.severity = error # RS0017: Remove deleted types and members from the declared API
+dotnet_diagnostic.RS0024.severity = error # RS0024: The contents of the public API files are invalid
+dotnet_diagnostic.RS0048.severity = error # RS0048: A missing API file turns the checks above off silently

ClickHouse.Driver.Common/ClickHouse.Driver.Common.csproj

@@ -39,6 +39,16 @@
   </ItemGroup>
 
   <ItemGroup>
+    <!-- The analyzer only reads files it is handed; it does not glob for them. -->
+    <AdditionalFiles Include="PublicAPI/PublicAPI.Shipped.txt" />
+    <AdditionalFiles Include="PublicAPI/PublicAPI.Unshipped.txt" />
+  </ItemGroup>
+
+  <ItemGroup>
+    <PackageReference Include="Microsoft.CodeAnalysis.PublicApiAnalyzers">
+      <PrivateAssets>all</PrivateAssets>
+      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
+    </PackageReference>
     <PackageReference Include="StyleCop.Analyzers">
       <PrivateAssets>all</PrivateAssets>
       <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>

ClickHouse.Driver.Tcp.Tests/ClickHouse.Driver.Tcp.Tests.csproj

@@ -23,6 +23,8 @@
       <PrivateAssets>all</PrivateAssets>
       <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
     </PackageReference>
+    <!-- ServiceCollection and BuildServiceProvider, for the IServiceCollection extensions. -->
+    <PackageReference Include="Microsoft.Extensions.DependencyInjection" />
     <PackageReference Include="NSubstitute" />
     <PackageReference Include="NUnit" />
     <PackageReference Include="NUnit.Analyzers">

ClickHouse.Driver.Tcp.Tests/Client/ClickHouseTcpClientOptionsTests.cs

@@ -577,7 +577,9 @@ public void With_ChangingOneProperty_CarriesEveryOtherPropertyAcross()
 
             foreach (PropertyInfo property in typeof(ClickHouseTcpClientOptions).GetProperties())
             {
-                if (property.Name == nameof(ClickHouseTcpClientOptions.Port))
+                // The property under change, and any property computed from it: ResolvedPort is derived from
+                // Port and UseTls, so it is meant to move when either does. Only stored state is carried across.
+                if (property.Name == nameof(ClickHouseTcpClientOptions.Port) || property.SetMethod is null)
                 {
                     continue;
                 }

ClickHouse.Driver.Tcp.Tests/Client/ClickHouseTcpServerInfoTests.cs

@@ -0,0 +1,52 @@
+using System;
+
+namespace ClickHouse.Driver.Tcp.Tests.Client;
+
+// Only what a live server cannot show: the derived Version and ToString, and that a patch the server never
+// sent still yields a three-part version rather than throwing.
+[TestFixture]
+public class ClickHouseTcpServerInfoTests
+{
+    [Test]
+    public void Version_ComposesTheThreeParts()
+    {
+        var info = new ClickHouseTcpServerInfo { VersionMajor = 25, VersionMinor = 8, VersionPatch = 3 };
+
+        Assert.That(info.Version, Is.EqualTo(new Version(25, 8, 3)));
+    }
+
+    [Test]
+    public void Version_PatchNotSent_IsZeroRatherThanUnset()
+    {
+        // The handshake omits the patch below the revision that introduced it, so it defaults to 0. A
+        // two-part Version would compare unequal to a three-part one, so it has to stay three parts.
+        var info = new ClickHouseTcpServerInfo { VersionMajor = 25, VersionMinor = 8 };
+
+        Assert.Multiple(() =>
+        {
+            Assert.That(info.Version, Is.EqualTo(new Version(25, 8, 0)));
+            Assert.That(info.Version.Build, Is.EqualTo(0));
+        });
+    }
+
+    [Test]
+    public void ToString_RendersNameAndThreePartVersion()
+    {
+        var info = new ClickHouseTcpServerInfo { Name = "ClickHouse", VersionMajor = 25, VersionMinor = 8, VersionPatch = 3 };
+
+        Assert.That(info.ToString(), Is.EqualTo("ClickHouse 25.8.3"));
+    }
+
+    [Test]
+    public void TimezoneAndDisplayName_DefaultToEmptyRatherThanNull()
+    {
+        // Both are blank when the negotiated revision predates them; empty keeps callers off a null check.
+        var info = new ClickHouseTcpServerInfo();
+
+        Assert.Multiple(() =>
+        {
+            Assert.That(info.Timezone, Is.Empty);
+            Assert.That(info.DisplayName, Is.Empty);
+        });
+    }
+}

ClickHouse.Driver.Tcp.Tests/DependencyInjection/ClickHouseTcpServiceCollectionExtensionsTests.cs

@@ -0,0 +1,254 @@
+using System;
+using System.Linq;
+using System.Threading.Tasks;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Logging.Abstractions;
+using NSubstitute;
+
+namespace ClickHouse.Driver.Tcp.Tests.DependencyInjection;
+
+/// <summary>
+/// Covers the registrations themselves: what is registered, at which lifetime, and who owns the pool. A data
+/// source dials nothing until an operation runs, so none of this needs a server.
+/// </summary>
+[TestFixture]
+public class ClickHouseTcpServiceCollectionExtensionsTests
+{
+    private const string ConnectionString = "Host=clickhouse.invalid;Port=9123;Username=someone;Database=somewhere";
+
+    private static ClickHouseTcpClientOptions Options() => new() { Host = "clickhouse.invalid", Port = 9123 };
+
+    [Test]
+    public async Task AddClickHouseTcpDataSource_WithConnectionString_ConfiguresTheDataSourceFromIt()
+    {
+        await using ServiceProvider provider = new ServiceCollection()
+            .AddClickHouseTcpDataSource(ConnectionString)
+            .BuildServiceProvider();
+
+        ClickHouseTcpClientOptions options = provider.GetRequiredService<ClickHouseTcpDataSource>().Options;
+
+        Assert.Multiple(() =>
+        {
+            Assert.That(options.Host, Is.EqualTo("clickhouse.invalid"));
+            Assert.That(options.Port, Is.EqualTo(9123));
+            Assert.That(options.Username, Is.EqualTo("someone"));
+            Assert.That(options.Database, Is.EqualTo("somewhere"));
+        });
+    }
+
+    [Test]
+    public void AddClickHouseTcpDataSource_WithOptions_RegistersOnlySingletons()
+    {
+        IServiceCollection services = new ServiceCollection().AddClickHouseTcpDataSource(Options());
+
+        Assert.That(
+            services.Select(descriptor => (descriptor.ServiceType, descriptor.Lifetime)),
+            Is.EquivalentTo(new[]
+            {
+                (typeof(ClickHouseTcpDataSource), ServiceLifetime.Singleton),
+                (typeof(IClickHouseTcpDataSource), ServiceLifetime.Singleton),
+                (typeof(IClickHouseTcpClient), ServiceLifetime.Singleton),
+                (typeof(IClickHouseTcpOperations), ServiceLifetime.Singleton),
+            }));
+    }
+
+    [Test]
+    public async Task AddClickHouseTcpDataSource_WithOptions_ResolvesTheClientTheDataSourceOwns()
+    {
+        await using ServiceProvider provider = new ServiceCollection()
+            .AddClickHouseTcpDataSource(Options())
+            .BuildServiceProvider();
+
+        var dataSource = provider.GetRequiredService<ClickHouseTcpDataSource>();
+
+        Assert.Multiple(() =>
+        {
+            Assert.That(provider.GetRequiredService<ClickHouseTcpDataSource>(), Is.SameAs(dataSource));
+            Assert.That(provider.GetRequiredService<IClickHouseTcpClient>(), Is.SameAs(dataSource.GetClient()));
+            Assert.That(provider.GetRequiredService<IClickHouseTcpOperations>(), Is.SameAs(dataSource.GetClient()));
+
+            // The interface forwards to the same instance, so injecting either one gets the owner of the pool
+            // rather than a second data source the provider would also dispose.
+            Assert.That(provider.GetRequiredService<IClickHouseTcpDataSource>(), Is.SameAs(dataSource));
+        });
+    }
+
+    [Test]
+    public async Task AddClickHouseTcpDataSource_WithoutALoggerFactoryInTheOptions_TakesTheProviderOne()
+    {
+        await using ServiceProvider provider = new ServiceCollection()
+            .AddSingleton<ILoggerFactory>(NullLoggerFactory.Instance)
+            .AddClickHouseTcpDataSource(Options())
+            .BuildServiceProvider();
+
+        ClickHouseTcpDataSource dataSource = provider.GetRequiredService<ClickHouseTcpDataSource>();
+
+        Assert.That(dataSource.Options.LoggerFactory, Is.SameAs(NullLoggerFactory.Instance));
+    }
+
+    [Test]
+    public async Task AddClickHouseTcpDataSource_WithALoggerFactoryInTheOptions_KeepsIt()
+    {
+        var configured = Substitute.For<ILoggerFactory>();
+
+        await using ServiceProvider provider = new ServiceCollection()
+            .AddSingleton<ILoggerFactory>(NullLoggerFactory.Instance)
+            .AddClickHouseTcpDataSource(Options() with { LoggerFactory = configured })
+            .BuildServiceProvider();
+
+        ClickHouseTcpDataSource dataSource = provider.GetRequiredService<ClickHouseTcpDataSource>();
+
+        Assert.That(dataSource.Options.LoggerFactory, Is.SameAs(configured));
+    }
+
+    [Test]
+    public async Task AddClickHouseTcpDataSource_WithAnOptionsFactory_RunsItOnceWithTheProvider()
+    {
+        int calls = 0;
+
+        await using ServiceProvider provider = new ServiceCollection()
+            .AddSingleton<ILoggerFactory>(NullLoggerFactory.Instance)
+            .AddClickHouseTcpDataSource(serviceProvider =>
+            {
+                calls++;
+                return Options() with { Database = serviceProvider.GetRequiredService<ILoggerFactory>().GetType().Name };
+            })
+            .BuildServiceProvider();
+
+        ClickHouseTcpDataSource dataSource = provider.GetRequiredService<ClickHouseTcpDataSource>();
+        _ = provider.GetRequiredService<IClickHouseTcpClient>();
+
+        Assert.Multiple(() =>
+        {
+            Assert.That(dataSource.Options.Database, Is.EqualTo(nameof(NullLoggerFactory)));
+            Assert.That(calls, Is.EqualTo(1));
+        });
+    }
+
+    [Test]
+    public async Task AddClickHouseTcpDataSource_WithADataSourceFactory_UsesWhatTheFactoryReturns()
+    {
+        var built = new ClickHouseTcpDataSource(Options() with { Database = "from_factory" });
+
+        await using ServiceProvider provider = new ServiceCollection()
+            .AddClickHouseTcpDataSource((_, _) => built)
+            .BuildServiceProvider();
+
+        Assert.That(provider.GetRequiredService<ClickHouseTcpDataSource>(), Is.SameAs(built));
+    }
+
+    [Test]
+    public async Task AddClickHouseTcpDataSource_WithAServiceKey_RegistersKeyedServicesOnly()
+    {
+        await using ServiceProvider provider = new ServiceCollection()
+            .AddClickHouseTcpDataSource(Options(), serviceKey: "reporting")
+            .BuildServiceProvider();
+
+        var dataSource = provider.GetRequiredKeyedService<ClickHouseTcpDataSource>("reporting");
+
+        Assert.Multiple(() =>
+        {
+            Assert.That(provider.GetRequiredKeyedService<IClickHouseTcpDataSource>("reporting"), Is.SameAs(dataSource));
+            Assert.That(provider.GetRequiredKeyedService<IClickHouseTcpClient>("reporting"), Is.SameAs(dataSource.GetClient()));
+            Assert.That(provider.GetRequiredKeyedService<IClickHouseTcpOperations>("reporting"), Is.SameAs(dataSource.GetClient()));
+            Assert.That(provider.GetService<ClickHouseTcpDataSource>(), Is.Null);
+            Assert.That(provider.GetService<IClickHouseTcpDataSource>(), Is.Null);
+            Assert.That(provider.GetService<IClickHouseTcpClient>(), Is.Null);
+        });
+    }
+
+    [Test]
+    public async Task AddClickHouseTcpDataSource_WithTwoServiceKeys_KeepsThePoolsApart()
+    {
+        await using ServiceProvider provider = new ServiceCollection()
+            .AddClickHouseTcpDataSource(Options() with { Database = "first" }, serviceKey: "first")
+            .AddClickHouseTcpDataSource(Options() with { Database = "second" }, serviceKey: "second")
+            .BuildServiceProvider();
+
+        var first = provider.GetRequiredKeyedService<ClickHouseTcpDataSource>("first");
+        var second = provider.GetRequiredKeyedService<ClickHouseTcpDataSource>("second");
+
+        Assert.Multiple(() =>
+        {
+            Assert.That(first.Options.Database, Is.EqualTo("first"));
+            Assert.That(second.Options.Database, Is.EqualTo("second"));
+            Assert.That(provider.GetRequiredKeyedService<IClickHouseTcpClient>("second"), Is.SameAs(second.GetClient()));
+        });
+    }
+
+    [Test]
+    public async Task AddClickHouseTcpDataSource_CalledTwiceWithoutAKey_KeepsTheFirstRegistration()
+    {
+        await using ServiceProvider provider = new ServiceCollection()
+            .AddClickHouseTcpDataSource(Options() with { Database = "first" })
+            .AddClickHouseTcpDataSource(Options() with { Database = "second" })
+            .BuildServiceProvider();
+
+        Assert.That(provider.GetRequiredService<ClickHouseTcpDataSource>().Options.Database, Is.EqualTo("first"));
+    }
+
+    [Test]
+    public void AddClickHouseTcpDataSource_WithANullArgument_Throws()
+    {
+        Assert.Multiple(() =>
+        {
+            Assert.Throws<ArgumentNullException>(() => ClickHouseTcpServiceCollectionExtensions.AddClickHouseTcpDataSource(null, ConnectionString));
+            Assert.Throws<ArgumentNullException>(() => new ServiceCollection().AddClickHouseTcpDataSource((string)null));
+            Assert.Throws<ArgumentNullException>(() => new ServiceCollection().AddClickHouseTcpDataSource((ClickHouseTcpClientOptions)null));
+            Assert.Throws<ArgumentNullException>(() => new ServiceCollection().AddClickHouseTcpDataSource((Func<IServiceProvider, ClickHouseTcpClientOptions>)null));
+            Assert.Throws<ArgumentNullException>(() => new ServiceCollection().AddClickHouseTcpDataSource((Func<IServiceProvider, object, ClickHouseTcpDataSource>)null));
+        });
+    }
+
+    [Test]
+    public async Task DisposeAsync_OnTheProvider_ClosesThePoolOnceAndWithoutThrowing()
+    {
+        ServiceProvider provider = new ServiceCollection()
+            .AddClickHouseTcpDataSource(Options())
+            .BuildServiceProvider();
+
+        var dataSource = provider.GetRequiredService<ClickHouseTcpDataSource>();
+        IClickHouseTcpClient client = provider.GetRequiredService<IClickHouseTcpClient>();
+
+        // The container holds both the data source and the client it owns, so it disposes the same pool twice.
+        await provider.DisposeAsync();
+
+        Assert.Multiple(() =>
+        {
+            Assert.ThrowsAsync<ObjectDisposedException>(async () => await client.PingAsync());
+            Assert.DoesNotThrowAsync(async () => await dataSource.DisposeAsync());
+        });
+    }
+
+    [Test]
+    public void Dispose_OnTheProviderWithOnlyTheDataSourceResolved_ClosesThePool()
+    {
+        ServiceProvider provider = new ServiceCollection()
+            .AddClickHouseTcpDataSource(Options())
+            .BuildServiceProvider();
+
+        IClickHouseTcpClient client = provider.GetRequiredService<ClickHouseTcpDataSource>().GetClient();
+
+        provider.Dispose();
+
+        Assert.ThrowsAsync<ObjectDisposedException>(async () => await client.PingAsync());
+    }
+
+    [Test]
+    public void Dispose_OnTheProviderWithTheClientResolved_ClosesThePool()
+    {
+        // The container tracks the resolved client, and a synchronous ServiceProvider.Dispose() rejects a
+        // tracked service that offers only IAsyncDisposable — rejecting it instead of disposing the rest of
+        // its list. ClickHouseTcpClient.Dispose exists so that neither happens here.
+        ServiceProvider provider = new ServiceCollection()
+            .AddClickHouseTcpDataSource(Options())
+            .BuildServiceProvider();
+
+        using ClickHouseTcpDataSource dataSource = provider.GetRequiredService<ClickHouseTcpDataSource>();
+        IClickHouseTcpClient client = provider.GetRequiredService<IClickHouseTcpClient>();
+
+        Assert.DoesNotThrow(provider.Dispose);
+        Assert.ThrowsAsync<ObjectDisposedException>(async () => await client.PingAsync());
+    }
+}

ClickHouse.Driver.Tcp.Tests/Exceptions/ClickHouseTcpServerExceptionTests.cs

@@ -22,6 +22,26 @@ public void Code_ServerSentACodeThisClientDoesNotName_ReadsAsUnknownWithTheRawVa
         });
     }
 
+    // The shapes a live server does not send: a message without the prefix, one that only looks like it carries
+    // it, and an empty name. That the real prefix is present at all is asserted in the integration suite.
+    [TestCase("DB::Exception", "DB::Exception: it failed", "it failed")]
+    [TestCase("DB::Exception", "it failed", "it failed")]
+    [TestCase("DB::Exception", "DB::Exception", "DB::Exception")]
+    [TestCase("DB::Exception", "DB::ExceptionX: it failed", "DB::ExceptionX: it failed")]
+    [TestCase("DB::NetException", "DB::Exception: it failed", "DB::Exception: it failed")]
+    [TestCase("", "DB::Exception: it failed", "DB::Exception: it failed")]
+    [TestCase(null, "DB::Exception: it failed", "DB::Exception: it failed")]
+    public void Message_ServerRepeatedTheNameAtTheHeadOfTheText_ReportsTheTextWithoutIt(string name, string sent, string expected)
+    {
+        var exception = new ClickHouseTcpServerException(60, name, sent, "stack trace");
+
+        Assert.Multiple(() =>
+        {
+            Assert.That(exception.Message, Is.EqualTo(expected));
+            Assert.That(exception.Name, Is.EqualTo(name), "the class name is still reported, just not twice.");
+        });
+    }
+
     [TestCase(ClickHouseErrorCode.TimeoutExceeded)]
     [TestCase(ClickHouseErrorCode.TooManySimultaneousQueries)]
     [TestCase(ClickHouseErrorCode.NoFreeConnection)]