Skip to content

Remove connection options inheritance #4237

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mdaigle wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Remove connection options inheritance #4237

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
5d26307
Rename SqlConnectionString to SqlConnectionOptions
mdaigle Apr 24, 2026
cadd40a
Remove the DbConnectionOptions parent class.
mdaigle Apr 24, 2026
9ef831b
Fix netfx compilation issues.
mdaigle Apr 24, 2026
4ff134c
Add comments and missing using.
mdaigle Apr 24, 2026
6209356
Fix compilation issues.
mdaigle Apr 24, 2026
341f181
Fix compilation issues.
mdaigle Apr 24, 2026
439c6ef
Remove stale file.
mdaigle Apr 24, 2026
bf5931d
Clean up redundant casts.
mdaigle Apr 24, 2026
1133675
Fix unit tests.
mdaigle Apr 24, 2026
6d0afa0
Fix compilation issue.
mdaigle Apr 24, 2026
c55f6fb
Fix test connection string.
mdaigle Apr 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,7 @@ internal static ArgumentException InvalidArgumentLength(string argumentName, int

internal static Exception CreateSqlException(
SqlAuthenticationProviderException authException,
SqlConnectionString connectionOptions,
SqlConnectionOptions connectionOptions,
SqlConnectionInternal sender,
string username)
{
Expand Down
782 changes: 0 additions & 782 deletions ...icrosoft.Data.SqlClient/src/Microsoft/Data/Common/ConnectionString/DbConnectionOptions.cs
View file
Open in desktop

This file was deleted.

14 changes: 8 additions & 6 deletions ...oft.Data.SqlClient/src/Microsoft/Data/Common/ConnectionString/DbConnectionString.netfx.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@

namespace Microsoft.Data.Common
{
// @TODO: Theoretically this class could be replaced with SqlConnectionString.
// Note: This class exists exclusively for use in SqlClientPermission. It provides a stable serialization format when the permission set is serialized
Comment thread
paulmedynski marked this conversation as resolved.
// by Code Access Security (CAS). CAS itself is deprecated in all versions of .NET and .NET Framework, but it won't be dropped from .NET Framework.
// We should maintain this class and SqlClientPermission to ensure that existing CAS-based security checks continue to work correctly on .NET Framework.

[Serializable] // MDAC 83147
internal sealed class DbConnectionString
Expand Down Expand Up @@ -53,20 +55,20 @@ internal DbConnectionString(
string restrictions,
KeyRestrictionBehavior behavior,
IReadOnlyDictionary<string, string> synonyms)
Copy link
Copy Markdown
Contributor Author

@mdaigle mdaigle Apr 27, 2026

Choose a reason for hiding this comment

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

We don't need to have a constructor that accepts synonyms. The only value that is ever passed is SqlConnectionOptions.KeywordMap. I opted to leave this constructor as-is to reduce the disturbance to anyone who uses this via reflection. Modifying these CAS helper-classes isn't the focus of this PR. They support a deprecated feature and I just need them to compile

Copy link
Copy Markdown
Contributor

@paulmedynski paulmedynski Apr 28, 2026

Choose a reason for hiding this comment

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

Do we officially support customers reflecting into our internals?

Copy link
Copy Markdown
Contributor Author

@mdaigle mdaigle Apr 28, 2026

Choose a reason for hiding this comment

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

No, but it still happens :) I basically just want to forget that these classes even exist and never touch them.

: this(new DbConnectionOptions(value, synonyms), restrictions, behavior, synonyms, false)
: this(new SqlConnectionOptions(value), restrictions, behavior, synonyms, false)
{
// The IReadOnlyDictionary doesn't need to be cloned since it isn't shared with anything else
Comment thread
mdaigle marked this conversation as resolved.
}

internal DbConnectionString(DbConnectionOptions connectionOptions)
internal DbConnectionString(SqlConnectionOptions connectionOptions)
: this(connectionOptions, (string)null, KeyRestrictionBehavior.AllowOnly, null, true)
{
// used by DBDataPermission to convert from DbConnectionOptions to DbConnectionString
// used by DBDataPermission to convert from SqlConnectionOptions to DbConnectionString
// since backward compatibility requires Everett level classes
}

Comment thread
mdaigle marked this conversation as resolved.
private DbConnectionString(
DbConnectionOptions connectionOptions,
SqlConnectionOptions connectionOptions,
string restrictions,
KeyRestrictionBehavior behavior,
IReadOnlyDictionary<string, string> synonyms,
Expand Down Expand Up @@ -505,7 +507,7 @@ private static string[] ParseRestrictions(string restrictions, IReadOnlyDictiona
int startPosition = nextStartPosition;

string keyname; // since parsing restrictions ignores values, it doesn't matter if we use ODBC rules or OLEDB rules
nextStartPosition = DbConnectionOptions.GetKeyValuePair(restrictions, startPosition, buffer, false, out keyname, out _);
nextStartPosition = SqlConnectionOptions.GetKeyValuePair(restrictions, startPosition, buffer, false, out keyname, out _);
if (!string.IsNullOrEmpty(keyname))
{
#if DEBUG
Expand Down
11 changes: 5 additions & 6 deletions src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionClosed.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
using System.Diagnostics;
using System.Threading.Tasks;
using Microsoft.Data.Common;
using Microsoft.Data.Common.ConnectionString;
using Microsoft.Data.SqlClient;
using Microsoft.Data.SqlClient.ConnectionPool;

Expand Down Expand Up @@ -54,7 +53,7 @@ internal override bool TryOpenConnection(
DbConnection outerConnection,
SqlConnectionFactory connectionFactory,
TaskCompletionSource<DbConnectionInternal> retry,
DbConnectionOptions userOptions) =>
SqlConnectionOptions userOptions) =>
TryOpenConnectionInternal(outerConnection, connectionFactory, retry, userOptions);

/// <inheritdoc/>
Expand All @@ -71,7 +70,7 @@ internal override bool TryOpenConnection(
DbConnection outerConnection,
SqlConnectionFactory connectionFactory,
TaskCompletionSource<DbConnectionInternal> retry,
DbConnectionOptions userOptions)
SqlConnectionOptions userOptions)
=> throw ADP.ConnectionAlreadyOpen(State);
}

Expand Down Expand Up @@ -113,14 +112,14 @@ internal override bool TryReplaceConnection(
DbConnection outerConnection,
SqlConnectionFactory connectionFactory,
TaskCompletionSource<DbConnectionInternal> retry,
DbConnectionOptions userOptions) =>
SqlConnectionOptions userOptions) =>
TryOpenConnection(outerConnection, connectionFactory, retry, userOptions);

internal override bool TryOpenConnection(
DbConnection outerConnection,
SqlConnectionFactory connectionFactory,
TaskCompletionSource<DbConnectionInternal> retry,
DbConnectionOptions userOptions)
SqlConnectionOptions userOptions)
{
if (retry == null || !retry.Task.IsCompleted)
{
Expand Down Expand Up @@ -169,7 +168,7 @@ internal override bool TryReplaceConnection(
DbConnection outerConnection,
SqlConnectionFactory connectionFactory,
TaskCompletionSource<DbConnectionInternal> retry,
DbConnectionOptions userOptions) =>
SqlConnectionOptions userOptions) =>
TryOpenConnection(outerConnection, connectionFactory, retry, userOptions);
}
}
7 changes: 3 additions & 4 deletions src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
using System.Threading.Tasks;
using System.Transactions;
using Microsoft.Data.Common;
using Microsoft.Data.Common.ConnectionString;
using Microsoft.Data.SqlClient;
using Microsoft.Data.SqlClient.ConnectionPool;
using Microsoft.Data.SqlClient.Internal;
Expand Down Expand Up @@ -802,7 +801,7 @@ internal virtual bool TryOpenConnection(
DbConnection outerConnection,
SqlConnectionFactory connectionFactory,
TaskCompletionSource<DbConnectionInternal> retry,
DbConnectionOptions userOptions)
SqlConnectionOptions userOptions)
{
throw ADP.ConnectionAlreadyOpen(State);
}
Expand All @@ -811,7 +810,7 @@ internal virtual bool TryReplaceConnection(
DbConnection outerConnection,
SqlConnectionFactory connectionFactory,
TaskCompletionSource<DbConnectionInternal> retry,
DbConnectionOptions userOptions)
SqlConnectionOptions userOptions)
{
throw ADP.MethodNotImplemented();
}
Expand Down Expand Up @@ -898,7 +897,7 @@ protected bool TryOpenConnectionInternal(
DbConnection outerConnection,
SqlConnectionFactory connectionFactory,
TaskCompletionSource<DbConnectionInternal> retry,
DbConnectionOptions userOptions)
SqlConnectionOptions userOptions)
{
// ?->Connecting: prevent set_ConnectionString during Open
if (connectionFactory.SetInnerConnectionFrom(outerConnection, DbConnectionClosedConnecting.SingletonInstance, this))
Expand Down
8 changes: 4 additions & 4 deletions src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/ServerInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@ internal sealed class ServerInfo
/// <summary>
/// Initialize server info from connection options alone.
/// </summary>
internal ServerInfo(SqlConnectionString userOptions)
internal ServerInfo(SqlConnectionOptions userOptions)
: this(userOptions, userOptions.DataSource, userOptions.ServerSPN)
{ }

/// <summary>
/// Initialize server info from connection options, but override DataSource and ServerSPN
/// with given server name and server SPN.
/// </summary>
internal ServerInfo(SqlConnectionString userOptions, string serverName, string serverSpn)
internal ServerInfo(SqlConnectionOptions userOptions, string serverName, string serverSpn)
: this(userOptions, serverName)
{
ServerSPN = serverSpn;
Expand All @@ -33,7 +33,7 @@ internal ServerInfo(SqlConnectionString userOptions, string serverName, string s
/// Initialize server info from connection options, but override DataSource with given
/// server name.
/// </summary>
private ServerInfo(SqlConnectionString userOptions, string serverName)
private ServerInfo(SqlConnectionOptions userOptions, string serverName)
{
Debug.Assert(userOptions != null);
Debug.Assert(serverName != null, "server name should never be null");
Expand All @@ -56,7 +56,7 @@ private ServerInfo(SqlConnectionString userOptions, string serverName)
/// server name.
/// </summary>
internal ServerInfo(
SqlConnectionString userOptions,
SqlConnectionOptions userOptions,
RoutingInfo routing,
string preRoutingServerName,
string serverSpn)
Expand Down
26 changes: 13 additions & 13 deletions ...Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,18 +313,18 @@ internal class SqlConnectionInternal : DbConnectionInternal, IDisposable
/// new Login7 packet will always write out the new password (or a length of zero and no
/// bytes if not present).
/// - userConnectionOptions may be different to connectionOptions if the connection string
/// has been expanded (see SqlConnectionString.Expand)
/// has been expanded (see SqlConnectionOptions.Expand)
/// </remarks>
// @TODO: We really really need simplify what we pass into this. All these optional parameters need to go!
internal SqlConnectionInternal(
DbConnectionPoolIdentity identity,
SqlConnectionString connectionOptions,
SqlConnectionOptions connectionOptions,
SqlCredential credential,
DbConnectionPoolGroupProviderInfo providerInfo,
string newPassword,
SecureString newSecurePassword,
bool redirectedUserInstance,
SqlConnectionString userConnectionOptions = null,
SqlConnectionOptions userConnectionOptions = null,
SessionData reconnectSessionData = null,
bool applyTransientFaultHandling = false,
string accessToken = null,
Expand Down Expand Up @@ -508,7 +508,7 @@ internal Guid ClientConnectionId
/// <summary>
/// The connection options to be used for this connection.
/// </summary>
internal SqlConnectionString ConnectionOptions { get; }
internal SqlConnectionOptions ConnectionOptions { get; }

/// <summary>
/// The current database for this connection. Null if the connection is not open yet.
Expand Down Expand Up @@ -941,7 +941,7 @@ internal void CheckEnlistedTransactionBinding()

if (enlistedTransaction != null)
{
if (ConnectionOptions.TransactionBinding is SqlConnectionString.TransactionBindingEnum.ExplicitUnbind)
if (ConnectionOptions.TransactionBinding is SqlConnectionOptions.TransactionBindingEnum.ExplicitUnbind)
{
Transaction currentTransaction = Transaction.Current;
if (enlistedTransaction.TransactionInformation.Status != TransactionStatus.Active || !enlistedTransaction.Equals(currentTransaction))
Expand Down Expand Up @@ -1971,7 +1971,7 @@ internal override bool TryReplaceConnection(
DbConnection outerConnection,
SqlConnectionFactory connectionFactory,
TaskCompletionSource<DbConnectionInternal> retry,
DbConnectionOptions userOptions)
SqlConnectionOptions userOptions)
{
return TryOpenConnectionInternal(outerConnection, connectionFactory, retry, userOptions);
}
Expand Down Expand Up @@ -2233,7 +2233,7 @@ private void AttemptOneLogin(
// @TODO: Rename to meet naming conventions
private bool AttemptRetryADAuthWithTimeoutError(
SqlException sqlex,
SqlConnectionString connectionOptions, // @TODO: this is not used
SqlConnectionOptions connectionOptions, // @TODO: this is not used
TimeoutTimer timeout)
{
if (!_activeDirectoryAuthTimeoutRetryHelper.CanRetryWithSqlException(sqlex))
Expand Down Expand Up @@ -3140,7 +3140,7 @@ private void LoginNoFailover(
string newPassword,
SecureString newSecurePassword,
bool redirectedUserInstance,
SqlConnectionString connectionOptions,
SqlConnectionOptions connectionOptions,
SqlCredential credential,
TimeoutTimer timeout)
{
Expand Down Expand Up @@ -3464,7 +3464,7 @@ private void LoginWithFailover(
string newPassword,
SecureString newSecurePassword,
bool redirectedUserInstance,
SqlConnectionString connectionOptions,
SqlConnectionOptions connectionOptions,
SqlCredential credential, // @TODO: This isn't used anywhere
TimeoutTimer timeout)
{
Expand Down Expand Up @@ -3779,7 +3779,7 @@ private bool IsTransientError(SqlException exc)

private void OpenLoginEnlist(
TimeoutTimer timeout,
SqlConnectionString connectionOptions,
SqlConnectionOptions connectionOptions,
SqlCredential credential,
string newPassword,
SecureString newSecurePassword,
Expand Down Expand Up @@ -3905,7 +3905,7 @@ internal override void ResetConnection()
}
}

private void ResolveExtendedServerName(ServerInfo serverInfo, bool aliasLookup, SqlConnectionString options)
private void ResolveExtendedServerName(ServerInfo serverInfo, bool aliasLookup, SqlConnectionOptions options)
{
// @TODO: Invert to save on indentation
if (serverInfo.ExtendedServerName == null)
Expand Down Expand Up @@ -3946,7 +3946,7 @@ private void ResolveExtendedServerName(ServerInfo serverInfo, bool aliasLookup,
if (options.EnforceLocalHost)
{
// Verify LocalHost for |DataDirectory| usage
SqlConnectionString.VerifyLocalHostAndFixup(
SqlConnectionOptions.VerifyLocalHostAndFixup(
ref host,
Comment thread
mdaigle marked this conversation as resolved.
enforceLocalHost: true,
fixup: true);
Expand All @@ -3958,7 +3958,7 @@ private void ResolveExtendedServerName(ServerInfo serverInfo, bool aliasLookup,
}

#if NETFRAMEWORK
private bool ShouldDisableTnir(SqlConnectionString connectionOptions)
private bool ShouldDisableTnir(SqlConnectionOptions connectionOptions)
{
bool isAzureEndPoint = ADP.IsAzureSqlServerEndpoint(connectionOptions.DataSource);

Expand Down
9 changes: 4 additions & 5 deletions ...oft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
using System.Threading.Tasks;
using System.Transactions;
using Microsoft.Data.Common;
using Microsoft.Data.Common.ConnectionString;
using Microsoft.Data.ProviderBase;
using static Microsoft.Data.SqlClient.ConnectionPool.DbConnectionPoolState;
using Microsoft.Data.SqlClient.Internal;
Expand Down Expand Up @@ -225,7 +224,7 @@ public void PutObjectFromTransactedPool(DbConnectionInternal connection)
/// <inheritdoc />
public DbConnectionInternal ReplaceConnection(
DbConnection owningObject,
DbConnectionOptions userOptions,
SqlConnectionOptions userOptions,
DbConnectionInternal oldConnection)
{
throw new NotImplementedException();
Expand Down Expand Up @@ -283,7 +282,7 @@ public void TransactionEnded(Transaction transaction, DbConnectionInternal trans
public bool TryGetConnection(
DbConnection owningObject,
TaskCompletionSource<DbConnectionInternal>? taskCompletionSource,
DbConnectionOptions userOptions,
SqlConnectionOptions userOptions,
out DbConnectionInternal? connection)
{
var timeout = TimeSpan.FromSeconds(owningObject.ConnectionTimeout);
Expand Down Expand Up @@ -384,7 +383,7 @@ public bool TryGetConnection(
/// </exception>
private DbConnectionInternal? OpenNewInternalConnection(
DbConnection? owningConnection,
DbConnectionOptions userOptions,
SqlConnectionOptions userOptions,
CancellationToken cancellationToken)
{
cancellationToken.ThrowIfCancellationRequested();
Expand Down Expand Up @@ -513,7 +512,7 @@ private void RemoveConnection(DbConnectionInternal connection)
/// </exception>
private async Task<DbConnectionInternal> GetInternalConnection(
DbConnection owningConnection,
DbConnectionOptions userOptions,
SqlConnectionOptions userOptions,
bool async,
TimeSpan timeout)
{
Expand Down
7 changes: 3 additions & 4 deletions ...osoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/DbConnectionPoolGroup.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
using System.Diagnostics;
using System.Threading;
using Microsoft.Data.Common;
using Microsoft.Data.Common.ConnectionString;
using Microsoft.Data.ProviderBase;
using Microsoft.Data.SqlClient.Internal;

Expand All @@ -33,7 +32,7 @@ namespace Microsoft.Data.SqlClient.ConnectionPool

sealed internal class DbConnectionPoolGroup
{
private readonly DbConnectionOptions _connectionOptions;
private readonly SqlConnectionOptions _connectionOptions;
private readonly DbConnectionPoolKey _poolKey;
private readonly DbConnectionPoolGroupOptions _poolGroupOptions;
private ConcurrentDictionary<DbConnectionPoolIdentity, IDbConnectionPool> _poolCollection;
Expand All @@ -50,7 +49,7 @@ sealed internal class DbConnectionPoolGroup
private const int PoolGroupStateIdle = 2; // all pools are pruned via Clear
private const int PoolGroupStateDisabled = 4; // factory pool entry pruning method

internal DbConnectionPoolGroup(DbConnectionOptions connectionOptions, DbConnectionPoolKey key, DbConnectionPoolGroupOptions poolGroupOptions)
internal DbConnectionPoolGroup(SqlConnectionOptions connectionOptions, DbConnectionPoolKey key, DbConnectionPoolGroupOptions poolGroupOptions)
{
Debug.Assert(connectionOptions != null, "null connection options");

Expand All @@ -66,7 +65,7 @@ internal DbConnectionPoolGroup(DbConnectionOptions connectionOptions, DbConnecti
_state = PoolGroupStateActive;
}

internal DbConnectionOptions ConnectionOptions => _connectionOptions;
internal SqlConnectionOptions ConnectionOptions => _connectionOptions;

internal DbConnectionPoolKey PoolKey => _poolKey;

Expand Down
5 changes: 2 additions & 3 deletions ...Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IDbConnectionPool.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
using System.Data.Common;
using System.Threading.Tasks;
using System.Transactions;
using Microsoft.Data.Common.ConnectionString;
using Microsoft.Data.ProviderBase;

#nullable enable
Expand Down Expand Up @@ -123,7 +122,7 @@ internal interface IDbConnectionPool
/// <param name="userOptions">The user options to use if a new connection must be opened.</param>
/// <param name="connection">The retrieved connection will be passed out via this parameter.</param>
/// <returns>True if a connection was set in the out parameter, otherwise returns false.</returns>
bool TryGetConnection(DbConnection owningObject, TaskCompletionSource<DbConnectionInternal>? taskCompletionSource, DbConnectionOptions userOptions, out DbConnectionInternal? connection);
bool TryGetConnection(DbConnection owningObject, TaskCompletionSource<DbConnectionInternal>? taskCompletionSource, SqlConnectionOptions userOptions, out DbConnectionInternal? connection);

/// <summary>
/// Replaces the internal connection currently associated with owningObject with a new internal connection from the pool.
Expand All @@ -132,7 +131,7 @@ internal interface IDbConnectionPool
/// <param name="userOptions">The user options to use if a new connection must be opened.</param>
/// <param name="oldConnection">The internal connection currently associated with the owning object.</param>
/// <returns>A reference to the new DbConnectionInternal.</returns>
DbConnectionInternal ReplaceConnection(DbConnection owningObject, DbConnectionOptions userOptions, DbConnectionInternal oldConnection);
DbConnectionInternal ReplaceConnection(DbConnection owningObject, SqlConnectionOptions userOptions, DbConnectionInternal oldConnection);

/// <summary>
/// Returns an internal connection to the pool.
Expand Down
Loading
Loading