Skip to content
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.

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

[API Proposal]: System.Data.IAsyncDbConnection #81723

Closed
SoftStoneDevelop opened this issue Feb 6, 2023 · 7 comments
Closed

[API Proposal]: System.Data.IAsyncDbConnection #81723

SoftStoneDevelop opened this issue Feb 6, 2023 · 7 comments

Comments

@SoftStoneDevelop
Copy link

SoftStoneDevelop commented Feb 6, 2023

Background and motivation

System.Data.Common.DbConnection already contains methods such as BeginTransactionAsync, CloseAsync and other asynchronous versions of the System.Data.IDbConnection interface. It would be convenient to inherit DbConnection from System.Data.IAsyncDbConnection.

API Proposal

From IDbConnection we select the common:

    public interface IDbConnectionCommon
    {
        //
        // Summary:
        //     Gets or sets the string used to open a database.
        //
        // Returns:
        //     A string containing connection settings.
        string ConnectionString { get; set; }
        //
        // Summary:
        //     Gets the time to wait (in seconds) while trying to establish a connection before
        //     terminating the attempt and generating an error.
        //
        // Returns:
        //     The time (in seconds) to wait for a connection to open. The default value is
        //     15 seconds.
        int ConnectionTimeout { get; }
        //
        // Summary:
        //     Gets the name of the current database or the database to be used after a connection
        //     is opened.
        //
        // Returns:
        //     The name of the current database or the name of the database to be used once
        //     a connection is open. The default value is an empty string.
        string Database { get; }
        //
        // Summary:
        //     Gets the current state of the connection.
        //
        // Returns:
        //     One of the System.Data.ConnectionState values.
        ConnectionState State { get; }
    }

The IDbConnection interface then inherits from IDbConnectionCommon:

    //
    // Summary:
    //     Represents an open connection to a data source, and is implemented by .NET Framework
    //     data providers that access relational databases.
    public interface IDbConnection : IDbConnectionCommon, IDisposable
    {
        //
        // Summary:
        //     Begins a database transaction.
        //
        // Returns:
        //     An object representing the new transaction.
        IDbTransaction BeginTransaction();
        //
        // Summary:
        //     Begins a database transaction with the specified System.Data.IsolationLevel value.
        //
        // Parameters:
        //   il:
        //     One of the System.Data.IsolationLevel values.
        //
        // Returns:
        //     An object representing the new transaction.
        IDbTransaction BeginTransaction(IsolationLevel il);
        //
        // Summary:
        //     Changes the current database for an open Connection object.
        //
        // Parameters:
        //   databaseName:
        //     The name of the database to use in place of the current database.
        void ChangeDatabase(string databaseName);
        //
        // Summary:
        //     Closes the connection to the database.
        void Close();
        //
        // Summary:
        //     Creates and returns a Command object associated with the connection.
        //
        // Returns:
        //     A Command object associated with the connection.
        IDbCommand CreateCommand();
        //
        // Summary:
        //     Opens a database connection with the settings specified by the ConnectionString
        //     property of the provider-specific Connection object.
        void Open();
    }

The IAsyncDbConnection interface will be like this:

    public interface IAsyncDbConnection : IDbConnectionCommon, IAsyncDisposable
    {
        //
        // Summary:
        //     Asynchronously begins a database transaction.
        //
        // Parameters:
        //   cancellationToken:
        //     An optional token to cancel the asynchronous operation. The default value is
        //     System.Threading.CancellationToken.None.
        //
        // Returns:
        //     A task whose System.Threading.Tasks.Task`1.Result property is an object representing
        //     the new transaction.
        ValueTask<IDbTransaction> BeginTransactionAsync(CancellationToken cancellationToken = default);

        //
        // Summary:
        //     Asynchronously begins a database transaction.
        //
        // Parameters:
        //   isolationLevel:
        //     One of the enumeration values that specifies the isolation level for the transaction
        //     to use.
        //
        //   cancellationToken:
        //     An optional token to cancel the asynchronous operation. The default value is
        //     System.Threading.CancellationToken.None.
        //
        // Returns:
        //     A task whose System.Threading.Tasks.Task`1.Result property is an object representing
        //     the new transaction.
        ValueTask<IDbTransaction> BeginTransactionAsync(IsolationLevel isolationLevel, CancellationToken cancellationToken = default);

        //
        // Summary:
        //     Asynchronously changes the current database for an open connection.
        //
        // Parameters:
        //   databaseName:
        //     The name of the database for the connection to use.
        //
        //   cancellationToken:
        //     An optional token to cancel the asynchronous operation. The default value is
        //     System.Threading.CancellationToken.None.
        //
        // Returns:
        //     A task representing the asynchronous operation.
        Task ChangeDatabaseAsync(string databaseName, CancellationToken cancellationToken = default);

        //
        // Summary:
        //     Asynchronously closes the connection to the database.
        //
        // Returns:
        //     A System.Threading.Tasks.Task representing the asynchronous operation.
        Task CloseAsync();

        //
        // Summary:
        //     An asynchronous version of System.Data.Common.DbConnection.Open, which opens
        //     a database connection with the settings specified by the System.Data.Common.DbConnection.ConnectionString.
        //     This method invokes the virtual method System.Data.Common.DbConnection.OpenAsync(System.Threading.CancellationToken)
        //     with CancellationToken.None.
        //
        // Returns:
        //     A task representing the asynchronous operation.
        Task OpenAsync();

        //
        // Summary:
        //     This is the asynchronous version of System.Data.Common.DbConnection.Open. Providers
        //     should override with an appropriate implementation. The cancellation token can
        //     optionally be honored. The default implementation invokes the synchronous System.Data.Common.DbConnection.Open
        //     call and returns a completed task. The default implementation will return a cancelled
        //     task if passed an already cancelled cancellationToken. Exceptions thrown by Open
        //     will be communicated via the returned Task Exception property. Do not invoke
        //     other methods and properties of the DbConnection object until the returned Task
        //     is complete.
        //
        // Parameters:
        //   cancellationToken:
        //     The cancellation instruction.
        //
        // Returns:
        //     A task representing the asynchronous operation.
        Task OpenAsync(CancellationToken cancellationToken);
    }

Then the DbConnection inheritance will be like this::

public abstract class DbConnection : Component, IDbConnection, IDisposable, IAsyncDbConnection, IAsyncDisposable

API Usage

    public async void SomeMethod(IAsyncDbConnection dbConnection)
    {
        await dbConnection.OpenAsync();
        await using (dbConnection)
        {
            await using (var transaction = await dbConnection.BeginTransactionAsync())
            {
                //do something
            }
        }
    }

Alternative Designs

Do not allocate the common IDbConnectionCommon interface, but duplicate its elements in IAsyncDbConnection. This way the IDbConnection will not be changed.

Risks

No response

@SoftStoneDevelop SoftStoneDevelop added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Feb 6, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Feb 6, 2023
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@SoftStoneDevelop
Copy link
Author

And, since it is supposed to change interfaces, why are the GetSchema methods not included in IDbConnection?

@ghost
Copy link

ghost commented Feb 6, 2023

Tagging subscribers to this area: @roji, @ajcvickers
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

System.Data.Common.DbConnection already contains methods such as BeginTransactionAsync, CloseAsync and other asynchronous versions of the System.Data.IDbConnection interface. It would be convenient to inherit DbConnection from System.Data.IAsyncDbConnection.

API Proposal

From IDbConnection we select the common:

    public interface IDbConnectionCommon
    {
        //
        // Summary:
        //     Gets or sets the string used to open a database.
        //
        // Returns:
        //     A string containing connection settings.
        string ConnectionString { get; set; }
        //
        // Summary:
        //     Gets the time to wait (in seconds) while trying to establish a connection before
        //     terminating the attempt and generating an error.
        //
        // Returns:
        //     The time (in seconds) to wait for a connection to open. The default value is
        //     15 seconds.
        int ConnectionTimeout { get; }
        //
        // Summary:
        //     Gets the name of the current database or the database to be used after a connection
        //     is opened.
        //
        // Returns:
        //     The name of the current database or the name of the database to be used once
        //     a connection is open. The default value is an empty string.
        string Database { get; }
        //
        // Summary:
        //     Gets the current state of the connection.
        //
        // Returns:
        //     One of the System.Data.ConnectionState values.
        ConnectionState State { get; }
    }

The IDbConnection interface then inherits from IDbConnectionCommon:

    //
    // Summary:
    //     Represents an open connection to a data source, and is implemented by .NET Framework
    //     data providers that access relational databases.
    public interface IDbConnection : IDbConnectionCommon, IDisposable
    {
        //
        // Summary:
        //     Begins a database transaction.
        //
        // Returns:
        //     An object representing the new transaction.
        IDbTransaction BeginTransaction();
        //
        // Summary:
        //     Begins a database transaction with the specified System.Data.IsolationLevel value.
        //
        // Parameters:
        //   il:
        //     One of the System.Data.IsolationLevel values.
        //
        // Returns:
        //     An object representing the new transaction.
        IDbTransaction BeginTransaction(IsolationLevel il);
        //
        // Summary:
        //     Changes the current database for an open Connection object.
        //
        // Parameters:
        //   databaseName:
        //     The name of the database to use in place of the current database.
        void ChangeDatabase(string databaseName);
        //
        // Summary:
        //     Closes the connection to the database.
        void Close();
        //
        // Summary:
        //     Creates and returns a Command object associated with the connection.
        //
        // Returns:
        //     A Command object associated with the connection.
        IDbCommand CreateCommand();
        //
        // Summary:
        //     Opens a database connection with the settings specified by the ConnectionString
        //     property of the provider-specific Connection object.
        void Open();
    }

The IAsyncDbConnection interface will be like this:

    public interface IAsyncDbConnection : IDbConnectionCommon, IAsyncDisposable
    {
        //
        // Summary:
        //     Asynchronously begins a database transaction.
        //
        // Parameters:
        //   cancellationToken:
        //     An optional token to cancel the asynchronous operation. The default value is
        //     System.Threading.CancellationToken.None.
        //
        // Returns:
        //     A task whose System.Threading.Tasks.Task`1.Result property is an object representing
        //     the new transaction.
        ValueTask<DbTransaction> BeginTransactionAsync(CancellationToken cancellationToken = default);

        //
        // Summary:
        //     Asynchronously begins a database transaction.
        //
        // Parameters:
        //   isolationLevel:
        //     One of the enumeration values that specifies the isolation level for the transaction
        //     to use.
        //
        //   cancellationToken:
        //     An optional token to cancel the asynchronous operation. The default value is
        //     System.Threading.CancellationToken.None.
        //
        // Returns:
        //     A task whose System.Threading.Tasks.Task`1.Result property is an object representing
        //     the new transaction.
        ValueTask<DbTransaction> BeginTransactionAsync(IsolationLevel isolationLevel, CancellationToken cancellationToken = default);

        //
        // Summary:
        //     Asynchronously changes the current database for an open connection.
        //
        // Parameters:
        //   databaseName:
        //     The name of the database for the connection to use.
        //
        //   cancellationToken:
        //     An optional token to cancel the asynchronous operation. The default value is
        //     System.Threading.CancellationToken.None.
        //
        // Returns:
        //     A task representing the asynchronous operation.
        Task ChangeDatabaseAsync(string databaseName, CancellationToken cancellationToken = default);

        //
        // Summary:
        //     Asynchronously closes the connection to the database.
        //
        // Returns:
        //     A System.Threading.Tasks.Task representing the asynchronous operation.
        Task CloseAsync();

        //
        // Summary:
        //     An asynchronous version of System.Data.Common.DbConnection.Open, which opens
        //     a database connection with the settings specified by the System.Data.Common.DbConnection.ConnectionString.
        //     This method invokes the virtual method System.Data.Common.DbConnection.OpenAsync(System.Threading.CancellationToken)
        //     with CancellationToken.None.
        //
        // Returns:
        //     A task representing the asynchronous operation.
        Task OpenAsync();

        //
        // Summary:
        //     This is the asynchronous version of System.Data.Common.DbConnection.Open. Providers
        //     should override with an appropriate implementation. The cancellation token can
        //     optionally be honored. The default implementation invokes the synchronous System.Data.Common.DbConnection.Open
        //     call and returns a completed task. The default implementation will return a cancelled
        //     task if passed an already cancelled cancellationToken. Exceptions thrown by Open
        //     will be communicated via the returned Task Exception property. Do not invoke
        //     other methods and properties of the DbConnection object until the returned Task
        //     is complete.
        //
        // Parameters:
        //   cancellationToken:
        //     The cancellation instruction.
        //
        // Returns:
        //     A task representing the asynchronous operation.
        Task OpenAsync(CancellationToken cancellationToken);
    }

Then the DbConnection inheritance will be like this::

public abstract class DbConnection : Component, IDbConnection, IDisposable, IAsyncDbConnection, IAsyncDisposable

API Usage

    public async void SomeMethod(IAsyncDbConnection dbConnection)
    {
        await dbConnection.OpenAsync();
        await using (dbConnection)
        {
            await using (var transaction = await dbConnection.BeginTransactionAsync())
            {
                //do something
            }
        }
    }

Alternative Designs

Do not allocate the common IDbConnectionCommon interface, but duplicate its elements in IAsyncDbConnection. This way the IDbConnection will not be changed.

Risks

No response

Author: SoftStoneDevelop
Assignees: -
Labels:

api-suggestion, area-System.Data, untriaged

Milestone: -

@roji
Copy link
Member

roji commented Feb 10, 2023

@SoftStoneDevelop we do not intend to evolve System.Data interfaces by adding additional ones for async support. For one thing, other non-async APIs have been added that would require yet other interfaces, causing an interface explosion.

C# now supports default interface implementations, which in theory could allow us to add the missing methods directly into e.g. IDbConnection without causing a breaking change. However, the System.Data interfaces have been more or less obsolete for a very long time, precisely because they could not be evolved; instead, users use the abstract base classes (DbConnection instead of IDbConnection). Is there any specific reason you're not simply using those?

@SoftStoneDevelop
Copy link
Author

@roji
Of course I can use DbConnection. It's just that IDbConnection is not marked as [Obsolete] because it assumed that either IAsyncDbConnection must exist or IDbConnection needs to be extended.

Because now I pass IDbConnection to methods that support different providers (so as not to be tied to a specific provider) and I have to additionally cast it to DbConnection:

if(connection is DbConnection dbConnection)
{
    //...
}

if the method is asynchronous.

Do all providers written for various .Net relational databases inherit from DbConnection and not just implement the IDbConnection interface? Are there any recommendations that are followed in this regard? It seemed more obvious to me only the obligation to implement the interface.

@roji
Copy link
Member

roji commented Feb 10, 2023

It's just that IDbConnection is not marked as [Obsolete] because it assumed that either IAsyncDbConnection must exist or IDbConnection needs to be extended.

That's not really how it works; marking an API as obsolete is quite an extreme thing, which causes compilation warnings in applications using them (and errors where warnings-as-errors is enabled). IDbConnection isn't obsolete - it simply isn't being evolved.

Do all providers written for various .Net relational databases inherit from DbConnection and not just implement the IDbConnection interface?

Yes. The recommendation is simply to use the abstract base classes instead of the interfaces.

@SoftStoneDevelop
Copy link
Author

In this case, I have no questions - I will use an abstract class.
If no one else has objections, then I suppose the issue can be closed.

@roji roji closed this as not planned Won't fix, can't repro, duplicate, stale Feb 10, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Feb 10, 2023
@roji roji removed the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Feb 10, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Mar 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants