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.

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

Jump to bottom

Add template methods around DbConnection inside RelationalConnection #22849

Merged
merged 7 commits into from
Oct 5, 2020
Merged

Add template methods around DbConnection inside RelationalConnection #22849

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0695db1
Enable overriding DbConnection.BeginTransaction
joaopgrassi Sep 29, 2020
87dedd5
Enable overriding DbConnection.Close
joaopgrassi Sep 29, 2020
5b201e8
Enable overriding DbConnection.EnlistTransaction
joaopgrassi Sep 29, 2020
65266b0
Enable overriding DbConnection.Dispose
joaopgrassi Sep 29, 2020
d6e4de5
Enable overriding DbConnection.State
joaopgrassi Sep 29, 2020
7b5986c
Comply with notnull api consistency
joaopgrassi Sep 30, 2020
532fa0e
Comply with async api consistency
joaopgrassi Sep 30, 2020
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
95 changes: 79 additions & 16 deletions src/EFCore.Relational/Storage/RelationalConnection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,11 +227,19 @@ public virtual void EnlistTransaction(Transaction transaction)
Dependencies.TransactionLogger.ExplicitTransactionEnlisted(this, transaction);
}

DbConnection.EnlistTransaction(transaction);
ConnectionEnlistTransaction(transaction);
Copy link
Member

Choose a reason for hiding this comment

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

@dotnet/efteam Any thoughts on naming these template methods before merging? (We will review in API review later anyway.)


EnlistedTransaction = transaction;
}

/// <summary>
/// Template method that by default calls <see cref="System.Data.Common.DbConnection.EnlistTransaction" /> but can be overriden
/// by providers to make a different call instead.
/// </summary>
/// <param name="transaction"> The transaction to be used. </param>
protected virtual void ConnectionEnlistTransaction([NotNull] Transaction transaction)
=> DbConnection.EnlistTransaction(transaction);

/// <summary>
/// Indicates whether the store connection supports ambient transactions
/// </summary>
Expand Down Expand Up @@ -301,7 +309,7 @@ public virtual IDbContextTransaction BeginTransaction(IsolationLevel isolationLe

var dbTransaction = interceptionResult.HasResult
? interceptionResult.Result
: DbConnection.BeginTransaction(isolationLevel);
: ConnectionBeginTransation(isolationLevel);

dbTransaction = Dependencies.TransactionLogger.TransactionStarted(
this,
Expand All @@ -313,6 +321,15 @@ public virtual IDbContextTransaction BeginTransaction(IsolationLevel isolationLe
return CreateRelationalTransaction(dbTransaction, transactionId, true);
}

/// <summary>
/// Template method that by default calls <see cref="System.Data.Common.DbConnection.BeginDbTransaction" /> but can be overriden
/// by providers to make a different call instead.
/// </summary>
/// <param name="isolationLevel"> The isolation level to use for the transaction. </param>
/// <returns> The newly created transaction. </returns>
protected virtual DbTransaction ConnectionBeginTransation(IsolationLevel isolationLevel)
=> DbConnection.BeginTransaction(isolationLevel);

/// <summary>
/// Asynchronously begins a new transaction.
/// </summary>
Expand Down Expand Up @@ -344,7 +361,7 @@ public virtual async Task<IDbContextTransaction> BeginTransactionAsync(

var dbTransaction = interceptionResult.HasResult
? interceptionResult.Result
: await DbConnection.BeginTransactionAsync(isolationLevel, cancellationToken).ConfigureAwait(false);
: await ConnectionBeginTransationAsync(isolationLevel, cancellationToken).ConfigureAwait(false);

dbTransaction = await Dependencies.TransactionLogger.TransactionStartedAsync(
this,
Expand All @@ -358,6 +375,18 @@ public virtual async Task<IDbContextTransaction> BeginTransactionAsync(
return CreateRelationalTransaction(dbTransaction, transactionId, true);
}

/// <summary>
/// Template method that by default calls <see cref="System.Data.Common.DbConnection.BeginDbTransactionAsync" /> but can be overriden
/// by providers to make a different call instead.
/// </summary>
/// <param name="isolationLevel"> The isolation level to use for the transaction. </param>
/// <param name="cancellationToken"> A <see cref="CancellationToken" /> to observe while waiting for the task to complete. </param>
/// <returns> The newly created transaction. </returns>
protected virtual ValueTask<DbTransaction> ConnectionBeginTransationAsync(
IsolationLevel isolationLevel,
CancellationToken cancellationToken = default)
=> DbConnection.BeginTransactionAsync(isolationLevel, cancellationToken);

private void EnsureNoTransactions()
{
if (CurrentTransaction != null)
Expand Down Expand Up @@ -542,13 +571,13 @@ public virtual Task RollbackTransactionAsync(CancellationToken cancellationToken
/// <returns> <see langword="true" /> if the underlying connection was actually opened; <see langword="false" /> otherwise. </returns>
public virtual bool Open(bool errorsExpected = false)
{
if (DbConnection.State == ConnectionState.Broken)
if (DbConnectionState == ConnectionState.Broken)
{
DbConnection.Close();
CloseDbConnection();
}

var wasOpened = false;
if (DbConnection.State != ConnectionState.Open)
if (DbConnectionState != ConnectionState.Open)
{
CurrentTransaction?.Dispose();
ClearTransactions(clearAmbient: false);
Expand Down Expand Up @@ -576,13 +605,13 @@ public virtual bool Open(bool errorsExpected = false)
/// </returns>
public virtual async Task<bool> OpenAsync(CancellationToken cancellationToken, bool errorsExpected = false)
{
if (DbConnection.State == ConnectionState.Broken)
if (DbConnectionState == ConnectionState.Broken)
{
await DbConnection.CloseAsync().ConfigureAwait(false);
await CloseDbConnectionAsync().ConfigureAwait(false);
}

var wasOpened = false;
if (DbConnection.State != ConnectionState.Open)
if (DbConnectionState != ConnectionState.Open)
{
if (CurrentTransaction != null)
{
Expand Down Expand Up @@ -740,7 +769,7 @@ private void HandleAmbientTransactions()
Dependencies.TransactionLogger.AmbientTransactionEnlisted(this, current);
current.TransactionCompleted += HandleTransactionCompleted;

DbConnection.EnlistTransaction(current);
ConnectionEnlistTransaction(current);
_ambientTransactions.Push(current);
}

Expand Down Expand Up @@ -774,7 +803,7 @@ public virtual bool Close()
CurrentTransaction?.Dispose();
ClearTransactions(clearAmbient: false);

if (DbConnection.State != ConnectionState.Closed)
if (DbConnectionState != ConnectionState.Closed)
{
var startTime = DateTimeOffset.UtcNow;
var stopwatch = Stopwatch.StartNew();
Expand All @@ -785,7 +814,7 @@ public virtual bool Close()
{
if (!interceptionResult.IsSuppressed)
{
DbConnection.Close();
CloseDbConnection();
}

wasClosed = true;
Expand All @@ -806,6 +835,13 @@ public virtual bool Close()
return wasClosed;
}

/// <summary>
/// Template method that by default calls <see cref="System.Data.Common.DbConnection.Close" /> but can be overriden
/// by providers to make a different call instead.
/// </summary>
protected virtual void CloseDbConnection()
=> DbConnection.Close();

/// <summary>
/// Closes the connection to the database.
/// </summary>
Expand All @@ -826,7 +862,7 @@ public virtual async Task<bool> CloseAsync()

ClearTransactions(clearAmbient: false);

if (DbConnection.State != ConnectionState.Closed)
if (DbConnectionState != ConnectionState.Closed)
{
var startTime = DateTimeOffset.UtcNow;
var stopwatch = Stopwatch.StartNew();
Expand All @@ -838,7 +874,7 @@ public virtual async Task<bool> CloseAsync()
{
if (!interceptionResult.IsSuppressed)
{
await DbConnection.CloseAsync().ConfigureAwait(false);
await CloseDbConnectionAsync().ConfigureAwait(false);
}

wasClosed = true;
Expand Down Expand Up @@ -869,6 +905,19 @@ await Dependencies.ConnectionLogger.ConnectionErrorAsync(
return wasClosed;
}

/// <summary>
/// Template method that by default calls <see cref="M:System.Data.Common.DbConnection.CloseAsync" /> but can be overriden
/// by providers to make a different call instead.
/// </summary>
protected virtual Task CloseDbConnectionAsync()
=> DbConnection.CloseAsync();

/// <summary>
/// Template method that by default calls <see cref="M:System.Data.Common.DbConnection.State" /> but can be overriden
/// by providers to make a different call instead.
/// </summary>
protected virtual ConnectionState DbConnectionState => DbConnection.State;

private bool ShouldClose()
=> (_openedCount == 0
|| _openedCount > 0
Expand Down Expand Up @@ -903,13 +952,20 @@ public virtual void Dispose()
if (_connectionOwned
&& _connection != null)
{
DbConnection.Dispose();
DisposeDbConnection();
_connection = null;
_openedCount = 0;
_openedInternally = false;
}
}

/// <summary>
/// Template method that by default calls Dispose on <see cref="System.Data.Common.DbConnection" /> but can be overriden
/// by providers to make a different call instead.
/// </summary>
protected virtual void DisposeDbConnection()
=> DbConnection.Dispose();

/// <summary>
/// Performs application-defined tasks associated with freeing, releasing, or resetting unmanaged resources.
/// </summary>
Expand All @@ -925,10 +981,17 @@ public virtual async ValueTask DisposeAsync()
if (_connectionOwned
&& _connection != null)
{
await DbConnection.DisposeAsync().ConfigureAwait(false);
await DisposeDbConnectionAsync().ConfigureAwait(false);
_connection = null;
_openedCount = 0;
}
}

/// <summary>
/// Template method that by default calls <see cref="System.Data.Common.DbConnection.DisposeAsync" /> but can be overriden
/// by providers to make a different call instead.
/// </summary>
protected virtual ValueTask DisposeDbConnectionAsync()
=> DbConnection.DisposeAsync();
}
}
1 change: 1 addition & 0 deletions test/EFCore.Relational.Tests/RelationalApiConsistencyTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ public override
typeof(RelationalDatabaseFacadeExtensions).GetMethod(nameof(RelationalDatabaseFacadeExtensions.CloseConnectionAsync)),
typeof(IRelationalConnection).GetMethod(nameof(IRelationalConnection.CloseAsync)),
typeof(RelationalConnection).GetMethod(nameof(RelationalConnection.CloseAsync)),
typeof(RelationalConnection).GetMethod("CloseDbConnectionAsync", BindingFlags.NonPublic | BindingFlags.Instance),
typeof(DbConnectionInterceptor).GetMethod(nameof(DbConnectionInterceptor.ConnectionClosingAsync)),
typeof(DbConnectionInterceptor).GetMethod(nameof(DbConnectionInterceptor.ConnectionClosedAsync)),
typeof(IDbConnectionInterceptor).GetMethod(nameof(IDbConnectionInterceptor.ConnectionClosingAsync)),
Expand Down