Fix | PoolGroupProviderInfo Typing #3417
Conversation
Make pool provider info arguments in *ConnectionFactory methods strongly typed
There was a problem hiding this comment.
Pull Request Overview
This PR restores the abstract signatures for pool provider info methods on the NetFX side and strengthens the typing of poolGroupProviderInfo across both NetFX and NetCore SqlConnectionFactory implementations.
- Reverted
CreateConnectionPoolGroupProviderInfoin NetFX to its abstract form so callers pass a strongly typedDbConnectionPoolGroupProviderInfo. - Updated all
CreateConnectionoverloads in both NetFX and NetCore to acceptDbConnectionPoolGroupProviderInfoinstead ofobject.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs | Made CreateConnectionPoolProviderInfo and CreateConnectionPoolGroupProviderInfo abstract for strong typing. |
| src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs | Changed constructor parameter from object providerInfo to DbConnectionPoolGroupProviderInfo and reformatted base call. |
| src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs | Updated CreateConnection overrides to use DbConnectionPoolGroupProviderInfo, improved null‐propagation, and adjusted provider info propagation. |
| src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs | Mirrored strong-typing changes and reformatted CreateConnection call to use named arguments for clarity. |
Comments suppressed due to low confidence (2)
src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs:58
- Consider using 'as SqlConnection' instead of a direct cast to avoid an InvalidCastException if
owningConnectionis ever not a SqlConnection, matching the NetFX implementation.
SqlConnection sqlOwningConnection = (SqlConnection)owningConnection;
src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs:94
- [nitpick] Add an XML documentation comment for this abstract method to explain its purpose and parameters, improving the clarity of the public API.
internal abstract DbConnectionPoolProviderInfo CreateConnectionPoolProviderInfo(
src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs
Show resolved
Hide resolved
In System.Data.dll which had ODBC/OleDB/(once OracleClient) and SqlClient, the base classes for conn pool were being used to manage pools for all the providers. The code you are modifying was passthrough, which needed to pass around an object. |
src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs
Show resolved
Hide resolved
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3417 +/- ##
=========================================
+ Coverage 0 63.58% +63.58%
=========================================
Files 0 299 +299
Lines 0 65418 +65418
=========================================
+ Hits 0 41594 +41594
- Misses 0 23824 +23824
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Description
This PR does two main changes:
I thought it was a safe change to make, but I neglected to notice that the poolgroupproviderinfo was being passed in from the caller of the method. I noticed this issue while working on merging SqlConnectionFactory, since I made this change on the netfx version but not on the netcore version.
Not sure why these were always being passed in as object, but they are now being passed in as DbConnectionPoolGroupProviderInfo, which is what they should have been from the beginning.
Issues
No issue currently correspond to these changes.
Testing
Well, it still builds. Kinda disappointed the initial issue didn't cause CI failures.