Cleanup | SqlConnectionFactory <- DbConnectionFactory

[benrr101]

Description

This is the first PR in a bit of a fun experiment where we see if we can seriously clean up the codebase. According to @saurabh500 the codebase we've inherited dates back to the days when it was part of System.Data that not only contained the SqlClient but OdbcClient and even OracleClient. This is why there's Db* classes buried in the codebase that are passed around instead of the Sql* classes. In the best cases, these base implementations are fully abstract or just provide fallback implementations that throw. In the worst cases, they create confusing loops of implementations. And regardless, we are constantly passing them around as Db* and doing a lot of unnecessary checks and casts to the Sql* classes. These seemingly redundant base classes have a lot of unnecessary code and make the codebase as a whole more confusing. If we can do away with them, it would be idea. So, as an experiment, I decided SqlConnectionFactory might be a good place to see if we can simply merge the Db* class into the Sql* class.

As with most of my other merge project PRs, this one can be digested commit by commit (although be forewarned these ones are a bit stranger than usual due to protection levels). Important callouts have been added via my own review of the commit. Keep in mind although this could easily turn into a gigantic PR that removes all Db* classes, I've tried my best to be surgical and only eliminate the DbConnectionFactory class.

Issues

N/A

Testing

I'm still having trouble running test projects locally, so I have attempted to run them, but I can't guarantee I caught all the issues. CI will hopefully catch the remainder.

[Copilot]

Pull Request Overview

This PR replaces the legacy DbConnectionFactory abstractions with a single SqlConnectionFactory and cleans up related code, while also adding a new Timer overload.

• Swaps all DbConnectionFactory references for SqlConnectionFactory and updates method signatures accordingly.
• Introduces a TimeSpan-based overload for AdapterUtil.UnsafeCreateTimer.
• Changes all SqlConnectionFactory.SingletonInstance usages to the new SqlConnectionFactory.Instance and removes now-unused compile links.

Reviewed Changes

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

Show a summary per file

File	Description
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs	Switched to SqlConnectionFactory and updated pooled-connection call
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IDbConnectionPool.cs	Changed ConnectionFactory property to SqlConnectionFactory
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/DbConnectionPoolGroup.cs	Updated methods to accept SqlConnectionFactory
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs	Adjusted ConnectionFactory property type
src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs	Modified open/close schema methods to use SqlConnectionFactory
src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionClosed.cs	Updated overrides to accept SqlConnectionFactory
src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs	Added TimeSpan overload for UnsafeCreateTimer
src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs	Revised TryReplaceConnection override signature
src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnectionHelper.cs	Replaced SingletonInstance with Instance
src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnection.cs	Swapped all SingletonInstance calls for Instance
src/Microsoft.Data.SqlClient/netfx/src/Microsoft.Data.SqlClient.csproj	Removed DbConnectionFactory compile include
src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs	Revised TryReplaceConnection override signature
src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnectionHelper.cs	Replaced SingletonInstance with Instance
src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnection.cs	Swapped SingletonInstance usage for Instance
src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj	Removed DbConnectionFactory compile include

Comments suppressed due to low confidence (2)

src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs:136

• The new TimeSpan-based overload should be covered by unit tests to verify correct timer creation and disposal behavior under different intervals.

        internal static Timer UnsafeCreateTimer(TimerCallback callback, object state, TimeSpan dueTime, TimeSpan period)

src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs:133

• [nitpick] Consider adding XML doc comments to both overloads to clarify the expected units for the int parameters and explain that the new overload accepts TimeSpans.

        internal static Timer UnsafeCreateTimer(TimerCallback callback, object state, int dueTime, int period) =>

[paulmedynski]

I love removing unnecessary polymorphism! Just a few comments.