feat: add AddClickHouseClient extension method for DI registration#226
feat: add AddClickHouseClient extension method for DI registration#226nandor23 wants to merge 1 commit intoClickHouse:mainfrom
AddClickHouseClient extension method for DI registration#226Conversation
| using System; | ||
| using Microsoft.Extensions.DependencyInjection; | ||
|
|
||
| namespace ClickHouse.Driver; |
There was a problem hiding this comment.
DI extension in wrong namespace, duplicating existing class
High Severity
The AddClickHouseClient extension method is placed in namespace ClickHouse.Driver, but this project already has a ClickHouseServiceCollectionExtensions class in Microsoft.Extensions.DependencyInjection (in DependencyInjection/ClickHouseServiceCollectionExtensions.cs). The standard .NET pattern — and the existing project convention — is to put DI extensions in Microsoft.Extensions.DependencyInjection so they're auto-discoverable without extra using directives. This creates a confusing duplicate class name and makes AddClickHouseClient invisible to users unless they add using ClickHouse.Driver.
There was a problem hiding this comment.
The existing ClickHouseServiceCollectionExtensions seems unnecessarily complex. The library documentation states that ClickHouseClient is "thread-safe" and "designed for singleton use", suggesting it handles resource management internally. It also only works with .NET7 and above.
Given this design, DI extensions shouldn't need custom HttpClient management, factory lambdas, or warning suppressions. A simple registration approach should suffice
| if (string.IsNullOrWhiteSpace(connectionString)) | ||
| { | ||
| throw new ArgumentNullException(nameof(connectionString)); | ||
| } |
There was a problem hiding this comment.
Wrong exception type for empty/whitespace connection strings
Low Severity
The validation uses string.IsNullOrWhiteSpace which catches null, empty, and whitespace-only strings, but always throws ArgumentNullException. When connectionString is "" or " " (not null), throwing ArgumentNullException is semantically incorrect — ArgumentException is the appropriate type for non-null invalid arguments.


Summary
Provides a standard .NET pattern for registering
IClickHouseClientas a singleton in the dependency injection container, as mentioned in #225.Usage:
Checklist
Delete items not relevant to your PR:
Note
Low Risk
Small additive change limited to DI wiring plus unit tests; main risk is incorrect lifetime/validation behavior, which is covered by tests.
Overview
Adds a DI registration entrypoint via
IServiceCollection.AddClickHouseClient(connectionString), which validates the connection string and registersIClickHouseClientas a singleton backed byClickHouseClient.Introduces unit tests ensuring the service resolves as a singleton and that null/whitespace connection strings throw
ArgumentNullException.Written by Cursor Bugbot for commit 8c3d031. This will update automatically on new commits. Configure here.