Skip to content

Add HttpClient factory support for KookSocketConfig#41

Merged
gehongyan merged 4 commits into
masterfrom
rest-httpfactory
Feb 14, 2026
Merged

Add HttpClient factory support for KookSocketConfig#41
gehongyan merged 4 commits into
masterfrom
rest-httpfactory

Conversation

@gehongyan

@gehongyan gehongyan commented Feb 13, 2026

Copy link
Copy Markdown
Owner

No description provided.

Copilot AI review requested due to automatic review settings February 13, 2026 14:22
@github-actions

github-actions Bot commented Feb 13, 2026

Copy link
Copy Markdown

Test Results (All Platforms)

    9 files  ±0      9 suites  ±0   6s ⏱️ -1s
  379 tests ±0    379 ✅ ±0  0 💤 ±0  0 ❌ ±0 
1 140 runs  ±0  1 140 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 082428e. ± Comparison against base commit 7f33dad.

♻️ This comment has been updated with latest results.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds HttpClient factory support to enable integration with IHttpClientFactory from dependency injection frameworks. The changes allow users to provide a custom HttpClient factory function when configuring KookSocketConfig, which is useful for scenarios requiring centralized HTTP client configuration, custom message handlers, or resilience policies.

Changes:

  • Added a new overload of DefaultRestClientProvider.Create() that accepts Func<HttpClient> to support factory-provided HttpClient instances
  • Modified DefaultRestClient constructor to accept an optional httpClientFactory parameter and conditionally configure the HttpClient based on whether it's factory-provided
  • Updated the sample application to demonstrate HttpClient factory integration with IHttpClientFactory

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/Kook.Net.Rest/Net/DefaultRestClientProvider.cs Adds new Create overload accepting Func<HttpClient> factory parameter with proper documentation
src/Kook.Net.Rest/Net/DefaultRestClient.cs Modifies constructor to accept optional factory parameter and conditionally initializes HttpClient
samples/Kook.Net.Samples.TextCommands/Program.cs Demonstrates HttpClient factory integration using IHttpClientFactory from Microsoft.Extensions.DependencyInjection
Comments suppressed due to low confidence (1)

src/Kook.Net.Rest/Net/DefaultRestClient.cs:66

  • HttpClient instances created by IHttpClientFactory should not be disposed. The factory manages the lifetime of HttpClients and their underlying handlers for connection pooling and proper resource management. Disposing a factory-created HttpClient can lead to resource leaks and connection pool issues.

You should track whether the HttpClient was created by the factory and conditionally dispose only when it was created directly. Consider adding a field like private readonly bool _shouldDisposeClient and setting it based on whether httpClientFactory was null, then only call _client.Dispose() in the Dispose method when _shouldDisposeClient is true.

        if (httpClientFactory is not null)
            _client = httpClientFactory();
        else
        {
            _client = new HttpClient(new HttpClientHandler
            {
                AutomaticDecompression = DecompressionMethods.GZip | DecompressionMethods.Deflate,
                UseCookies = false,
                UseProxy = useProxy,
                Proxy = webProxy
            });
            SetHeader("accept-encoding", "gzip, deflate");
        }

        _cancellationToken = CancellationToken.None;

        _serializerOptions = new JsonSerializerOptions
        {
            Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping,
            NumberHandling = JsonNumberHandling.AllowReadingFromString
        };
    }

    private void Dispose(bool disposing)
    {
        if (!_isDisposed)
        {
            if (disposing)
                _client.Dispose();

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread samples/Kook.Net.Samples.TextCommands/Program.cs
Comment thread src/Kook.Net.Rest/Net/DefaultRestClientProvider.cs

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/Kook.Net.Rest/Net/DefaultRestClient.cs
Comment thread src/Kook.Net.Rest/Net/DefaultRestClientProvider.cs
Comment thread src/Kook.Net.Rest/Net/DefaultRestClientProvider.cs
Comment thread src/Kook.Net.Rest/Net/DefaultRestClientProvider.cs Outdated
@sonarqubecloud

Copy link
Copy Markdown

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@gehongyan gehongyan merged commit 9ffa52b into master Feb 14, 2026
15 checks passed
@gehongyan gehongyan deleted the rest-httpfactory branch February 14, 2026 03:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants