Skip to content

Remove extra connection options parameters#4261

Merged
mdaigle merged 6 commits into
mainfrom
dev/mdaigle/remove-extra-connection-options
May 7, 2026
Merged

Remove extra connection options parameters#4261
mdaigle merged 6 commits into
mainfrom
dev/mdaigle/remove-extra-connection-options

Conversation

@mdaigle

@mdaigle mdaigle commented May 6, 2026

Copy link
Copy Markdown
Contributor

This pull request refactors the connection management code to simplify method signatures and remove the use of the SqlConnectionOptions userOptions parameter from internal APIs and connection pool implementations. The main goal is to streamline the process of opening and replacing connections, making the codebase easier to maintain and less error-prone.

Connection Management API Simplification:

  • Removed the SqlConnectionOptions userOptions parameter from the signatures of TryOpenConnection, TryReplaceConnection, and related methods in DbConnectionInternal, DbConnectionClosed, and SqlConnectionInternal. All internal logic and calls have been updated accordingly. [1] [2] [3] [4] [5]
  • Updated the implementation of TryOpenConnectionInternal and its callers to no longer require or pass userOptions. [1] [2]

Connection Pool Interface and Implementation Updates:

  • Simplified the IDbConnectionPool interface by removing the userOptions parameter from TryGetConnection and ReplaceConnection, and updated all implementing classes (ChannelDbConnectionPool, WaitHandleDbConnectionPool) to match the new signatures. [1] [2] [3] [4] [5] [6]
  • Refactored all usages of connection pool methods to remove the now-unnecessary userOptions argument, including internal helper methods such as OpenNewInternalConnection and GetInternalConnection. [1] [2] [3] [4] [5]

Code Clean-up and Comments:

  • Removed obsolete comments and code related to userConnectionOptions, including .NET Framework-specific permission demand logic that is no longer relevant. [1] [2] [3]

New tests for CAS Functionality

  • Added tests to verify that the appropriate permissions are demanded when opening a connection. CAS is a deprecated feature, but we maintain it for backwards compatibility.

These changes collectively modernize and clarify the connection handling logic, reducing unnecessary complexity and improving maintainability.

Copilot AI review requested due to automatic review settings May 6, 2026 18:19
@github-project-automation github-project-automation Bot moved this to To triage in SqlClient Board May 6, 2026

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 pull request simplifies SqlClient’s internal connection-opening and pooling pathways by removing the redundant SqlConnectionOptions userOptions parameter from multiple internal methods/constructors, aligning call chains across pooled/non-pooled and sync/async flows.

Changes:

  • Removed userOptions from internal open/replace/get-connection APIs (DbConnectionInternal/DbConnectionClosed/SqlConnectionInternalSqlConnectionFactoryIDbConnectionPool implementations).
  • Updated IDbConnectionPool and both pool implementations (wait-handle + channel-based) to the new signatures.
  • Updated unit tests to use the simplified pool API surface.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolTransactionTest.cs Updates unit tests to call pool APIs without SqlConnectionOptions.
src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/TransactedConnectionPoolTest.cs Updates mock IDbConnectionPool implementation signatures to match interface changes.
src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs Updates tests to remove SqlConnectionOptions arguments from pool calls.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs Removes userOptions threading through connection creation/open paths and pool calls.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnection.cs Updates open logic to call TryOpenConnection/TryReplaceConnection without UserConnectionOptions.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs Removes userOptions from pending/open/create APIs and replaces usages with simplified calls.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IDbConnectionPool.cs Simplifies interface by removing userOptions from TryGetConnection and ReplaceConnection.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs Updates interface implementation and removes userOptions plumbing from internal open logic.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs Removes optional userConnectionOptions parameter and associated NETFRAMEWORK permission-demand block.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs Updates internal open pipeline to no longer pass userOptions into factory/pool methods.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionClosed.cs Updates closed-state open/replace paths to match the simplified method signatures.

@mdaigle mdaigle added this to the 7.1.0-preview2 milestone May 6, 2026
@mdaigle
mdaigle marked this pull request as ready for review May 6, 2026 21:07
@mdaigle
mdaigle requested a review from a team as a code owner May 6, 2026 21:07
Copilot AI review requested due to automatic review settings May 6, 2026 21:07

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 12 out of 12 changed files in this pull request and generated 3 comments.

Comment thread src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/CasTest.cs Outdated
Copilot AI review requested due to automatic review settings May 6, 2026 21:25

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 12 out of 12 changed files in this pull request and generated 1 comment.

@codecov

codecov Bot commented May 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.50000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.56%. Comparing base (be95ca2) to head (be09e9a).

Files with missing lines Patch % Lines
.../Microsoft/Data/ProviderBase/DbConnectionClosed.cs 33.33% 2 Missing ⚠️
...icrosoft/Data/ProviderBase/DbConnectionInternal.cs 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4261      +/-   ##
==========================================
- Coverage   65.96%   64.56%   -1.41%     
==========================================
  Files         275      270       -5     
  Lines       42993    65777   +22784     
==========================================
+ Hits        28361    42468   +14107     
- Misses      14632    23309    +8677     
Flag Coverage Δ
CI-SqlClient ?
PR-SqlClient-Project 64.56% <87.50%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mdaigle
mdaigle merged commit 4a36399 into main May 7, 2026
301 checks passed
@mdaigle
mdaigle deleted the dev/mdaigle/remove-extra-connection-options branch May 7, 2026 13:43
@github-project-automation github-project-automation Bot moved this from To triage to Done in SqlClient Board May 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants