Skip to content

Enforce IHttpClientFactory as required parameter in GetDefaultHttpClient#162

Merged
pontemonti merged 2 commits intousers/johanb/RealTimeThreatProtectionfrom
copilot/sub-pr-148-another-one
Dec 31, 2025
Merged

Enforce IHttpClientFactory as required parameter in GetDefaultHttpClient#162
pontemonti merged 2 commits intousers/johanb/RealTimeThreatProtectionfrom
copilot/sub-pr-148-another-one

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Dec 31, 2025

Addresses feedback from #148 to eliminate the optional fallback path that instantiates HttpClient directly, which can cause socket exhaustion.

Changes

  • RuntimeUtility.GetDefaultHttpClient: Made IHttpClientFactory parameter required (non-nullable), removed ?? new HttpClient() fallback
  • Documentation: Updated XML comments to reflect required parameter

Breaking Change

// Before: Optional factory with fallback
public static HttpClient GetDefaultHttpClient(
    IHttpClientFactory? httpClientFactory = null, 
    IUserAgentConfiguration? userAgentConfiguration = null, 
    int timeoutSeconds = 30)
{
    var httpClient = httpClientFactory?.CreateClient() ?? new HttpClient();
    // ...
}

// After: Factory required
public static HttpClient GetDefaultHttpClient(
    IHttpClientFactory httpClientFactory,
    IUserAgentConfiguration? userAgentConfiguration = null,
    int timeoutSeconds = 30)
{
    var httpClient = httpClientFactory.CreateClient();
    // ...
}

All existing call sites in McpToolServerConfigurationService already provide the factory via dependency injection.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>
Copilot AI changed the title [WIP] Update API implementation for sending chat history Enforce IHttpClientFactory as required parameter in GetDefaultHttpClient Dec 31, 2025
Copilot AI requested a review from pontemonti December 31, 2025 06:42
@pontemonti pontemonti marked this pull request as ready for review December 31, 2025 16:14
@pontemonti pontemonti requested a review from a team as a code owner December 31, 2025 16:14
@pontemonti pontemonti merged commit 3dd2273 into users/johanb/RealTimeThreatProtection Dec 31, 2025
1 check passed
@pontemonti pontemonti deleted the copilot/sub-pr-148-another-one branch December 31, 2025 16:14
pontemonti added a commit that referenced this pull request Jan 6, 2026
…148)

* Initial implementation of API to send chat history to MCP platform.

* Address PR feedback to clarify documentation.

* Address PR feedback about documentation

* Address PR feedback related to references to instance fields

* Address PR feedback related to documentation

* Address PR feedback about required vs optional properties

* Update src/Tooling/Core/Services/McpToolServerConfigurationService.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update src/Tooling/Core/Utils/Utility.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update src/Tests/Microsoft.Agents.A365.Tooling.Tests/Models/ChatHistoryMessageTests.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Use short-form Encoding.UTF8 instead of fully qualified name (#153)

* Initial plan

* Use Encoding.UTF8 instead of fully qualified name

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

* Document exception handling behavior in SendChatHistoryAsync XML comments (#152)

* Initial plan

* Update XML documentation for SendChatHistoryAsync methods to clarify exception handling

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

* Refactor HttpClient to use IHttpClientFactory to prevent socket exhaustion (#150)

* Initial plan

* Refactor HttpClient usage to use IHttpClientFactory

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

* Use RuntimeUtility.GetDefaultHttpClient with IHttpClientFactory

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

* Remove unnecessary using statement

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

* Add comprehensive test coverage for SendChatHistoryAsync parameter validation (#151)

* Initial plan

* Add comprehensive tests for SendChatHistoryAsync method

- Added tests for missing Activity properties (conversation ID, message ID, user message)
- All tests validate expected exception handling
- Tests verify that InvalidOperationException is thrown with appropriate error messages
- Tests cover both method overloads (with and without ToolOptions)
- Existing null parameter validation tests remain unchanged

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

* Fix tests to properly mock distinct Activity property scenarios

- SendChatHistoryAsync_MissingConversationId now properly mocks a null Conversation property
- SendChatHistoryAsync_MissingMessageId now mocks null Id with valid Conversation
- SendChatHistoryAsync_MissingUserMessage now mocks null Text with valid Conversation
- Each test validates a distinct error path through the method
- All 8 tests pass successfully

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

* Address code review feedback: improve code style

- Remove excessive blank lines between methods
- Use explicit cast syntax (Type)null! instead of null as Type!
- Maintain consistent null handling across all test methods
- All 8 tests continue to pass

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

* Update src/Tooling/Extensions/AgentFramework/Agent365AgentFrameworkSdkUserAgentConfiguration.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Fix missing IHttpClientFactory parameter in test constructor calls (#154)

* Initial plan

* Fix test failures by adding missing IHttpClientFactory parameter to McpToolServerConfigurationService constructor calls

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

* Remove exception throw for unsuccessful HTTP responses in SendChatHistoryAsync (#156)

* Initial plan

* Remove exception throw for failed HTTP requests in SendChatHistoryAsync

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

* Add happy path test coverage for SendChatHistoryAsync HTTP POST (#157)

* Initial plan

* Add comprehensive happy path tests for SendChatHistoryAsync method

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

* Fix flaky test behavior by using fixed timestamps instead of DateTimeOffset.UtcNow

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

* Update src/Tooling/Extensions/AzureAIFoundry/Agent365AzureAIFoundrySdkUserAgentConfiguration.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update src/Tooling/Extensions/SemanticKernel/Agent365SemanticKernelSdkUserAgentConfiguration.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update src/Tests/Microsoft.Agents.A365.Tooling.Tests/Services/McpToolServerConfigurationServiceTests.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update src/Tests/Microsoft.Agents.A365.Tooling.Tests/Services/McpToolServerConfigurationServiceTests.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update src/Tests/Microsoft.Agents.A365.Tooling.Tests/Services/McpToolServerConfigurationServiceTests.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Enforce IHttpClientFactory as required parameter in GetDefaultHttpClient (#162)

* Initial plan

* Make IHttpClientFactory required in GetDefaultHttpClient

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

* Add test coverage for non-success HTTP response scenarios in SendChatHistoryAsync (#161)

* Initial plan

* Add test coverage for non-success HTTP response scenarios (400, 401, 403, 404, 500)

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

* Refactor HTTP error status tests to use parameterized theory approach

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

* Update src/Tests/Microsoft.Agents.A365.Tooling.Tests/Services/McpToolServerConfigurationServiceTests.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update src/Tooling/Core/Models/ChatMessageRequest.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Fix HttpResponseMessage disposal in SendChatHistoryAsync (#163)

* Initial plan

* Wrap HttpResponseMessage in using statement for proper disposal

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

* Fix resource leak from undisposed HttpResponseMessage in test mocks (#164)

* Initial plan

* Fix HttpResponseMessage disposal issues by using lambda factory pattern

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

* Update src/Tests/Microsoft.Agents.A365.Tooling.Tests/Services/McpToolServerConfigurationServiceTests.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Fix HttpResponseMessage disposal race condition in async mock test (#165)

* Initial plan

* Fix HttpResponseMessage disposal race condition in test

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

* Redesign SendChatHistoryAsync to return a result object.

* Add CancellationToken argument

---------

Co-authored-by: Johan Broberg <johanb@microsoft.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>
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