Skip to content

Commit

Permalink
Remove cancellation token from Close APIs
Browse files Browse the repository at this point in the history
Fixes #16353
  • Loading branch information
ajcvickers committed Jul 2, 2019
1 parent f53e376 commit 32ff2d1
Show file tree
Hide file tree
Showing 15 changed files with 46 additions and 63 deletions.
8 changes: 2 additions & 6 deletions src/EFCore.Relational/Diagnostics/DbConnectionInterceptor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,6 @@ public virtual Task ConnectionOpenedAsync(
/// its implementation of this method.
/// This value is typically used as the return value for the implementation of this method.
/// </param>
/// <param name="cancellationToken"> The cancellation token. </param>
/// <returns>
/// If the <see cref="Task" /> result is null, then EF will close the connection as normal.
/// If the <see cref="Task" /> result is non-null value, then connection closing is suppressed.
Expand All @@ -129,8 +128,7 @@ public virtual Task ConnectionOpenedAsync(
public virtual Task<InterceptionResult?> ConnectionClosingAsync(
DbConnection connection,
ConnectionEventData eventData,
InterceptionResult? result,
CancellationToken cancellationToken = default)
InterceptionResult? result)
=> Task.FromResult(result);

/// <summary>
Expand All @@ -149,12 +147,10 @@ public virtual void ConnectionClosed(
/// </summary>
/// <param name="connection"> The connection. </param>
/// <param name="eventData"> Contextual information about the connection. </param>
/// <param name="cancellationToken"> The cancellation token. </param>
/// <returns> A <see cref="Task"/> representing the asynchronous operation. </returns>
public virtual Task ConnectionClosedAsync(
DbConnection connection,
ConnectionEndEventData eventData,
CancellationToken cancellationToken = default)
ConnectionEndEventData eventData)
=> Task.CompletedTask;

/// <summary>
Expand Down
8 changes: 2 additions & 6 deletions src/EFCore.Relational/Diagnostics/IDbConnectionInterceptor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,6 @@ Task ConnectionOpenedAsync(
/// its implementation of this method.
/// This value is typically used as the return value for the implementation of this method.
/// </param>
/// <param name="cancellationToken"> The cancellation token. </param>
/// <returns>
/// If the <see cref="Task" /> result is null, then EF will close the connection as normal.
/// If the <see cref="Task" /> result is non-null value, then connection closing is suppressed.
Expand All @@ -141,8 +140,7 @@ Task ConnectionOpenedAsync(
Task<InterceptionResult?> ConnectionClosingAsync(
[NotNull] DbConnection connection,
[NotNull] ConnectionEventData eventData,
InterceptionResult? result,
CancellationToken cancellationToken = default);
InterceptionResult? result);

/// <summary>
/// Called just after EF has called <see cref="DbConnection.Close()" /> in an async context.
Expand All @@ -158,12 +156,10 @@ void ConnectionClosed(
/// </summary>
/// <param name="connection"> The connection. </param>
/// <param name="eventData"> Contextual information about the connection. </param>
/// <param name="cancellationToken"> The cancellation token. </param>
/// <returns> A <see cref="Task"/> representing the asynchronous operation. </returns>
Task ConnectionClosedAsync(
[NotNull] DbConnection connection,
[NotNull] ConnectionEndEventData eventData,
CancellationToken cancellationToken = default);
[NotNull] ConnectionEndEventData eventData);

/// <summary>
/// Called when closing of a connection has failed with an exception. />.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,12 +100,11 @@ public async Task ConnectionOpenedAsync(
public async Task<InterceptionResult?> ConnectionClosingAsync(
DbConnection connection,
ConnectionEventData eventData,
InterceptionResult? result,
CancellationToken cancellationToken = default)
InterceptionResult? result)
{
for (var i = 0; i < _interceptors.Length; i++)
{
result = await _interceptors[i].ConnectionClosingAsync(connection, eventData, result, cancellationToken);
result = await _interceptors[i].ConnectionClosingAsync(connection, eventData, result);
}

return result;
Expand All @@ -123,12 +122,11 @@ public void ConnectionClosed(

public async Task ConnectionClosedAsync(
DbConnection connection,
ConnectionEndEventData eventData,
CancellationToken cancellationToken = default)
ConnectionEndEventData eventData)
{
for (var i = 0; i < _interceptors.Length; i++)
{
await _interceptors[i].ConnectionClosedAsync(connection, eventData, cancellationToken);
await _interceptors[i].ConnectionClosedAsync(connection, eventData);
}
}

Expand Down
12 changes: 4 additions & 8 deletions src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1290,13 +1290,11 @@ private static string ConnectionOpened(EventDefinitionBase definition, EventData
/// <param name="diagnostics"> The diagnostics logger to use. </param>
/// <param name="connection"> The connection. </param>
/// <param name="startTime"> The time that the operation was started. </param>
/// <param name="cancellationToken"> The cancellation token. </param>
/// <returns> A <see cref="Task"/> representing the async operation. </returns>
public static Task<InterceptionResult?> ConnectionClosingAsync(
[NotNull] this IDiagnosticsLogger<DbLoggerCategory.Database.Connection> diagnostics,
[NotNull] IRelationalConnection connection,
DateTimeOffset startTime,
CancellationToken cancellationToken = default)
DateTimeOffset startTime)
{
var definition = RelationalResources.LogClosingConnection(diagnostics);
Expand All @@ -1318,7 +1316,7 @@ private static string ConnectionOpened(EventDefinitionBase definition, EventData
if (interceptor != null)
{
return interceptor.ConnectionClosingAsync(connection.DbConnection, eventData, null, cancellationToken);
return interceptor.ConnectionClosingAsync(connection.DbConnection, eventData, null);
}
}
Expand Down Expand Up @@ -1417,14 +1415,12 @@ public static void ConnectionClosed(
/// <param name="connection"> The connection. </param>
/// <param name="startTime"> The time that the operation was started. </param>
/// <param name="duration"> The amount of time before the connection was closed. </param>
/// <param name="cancellationToken"> The cancellation token. </param>
/// <returns> A <see cref="Task"/> representing the async operation. </returns>
public static Task ConnectionClosedAsync(
[NotNull] this IDiagnosticsLogger<DbLoggerCategory.Database.Connection> diagnostics,
[NotNull] IRelationalConnection connection,
DateTimeOffset startTime,
TimeSpan duration,
CancellationToken cancellationToken = default)
TimeSpan duration)
{
var definition = RelationalResources.LogClosedConnection(diagnostics);
Expand All @@ -1447,7 +1443,7 @@ public static Task ConnectionClosedAsync(
if (interceptor != null)
{
return interceptor.ConnectionClosedAsync(connection.DbConnection, eventData, cancellationToken);
return interceptor.ConnectionClosedAsync(connection.DbConnection, eventData);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -703,10 +703,9 @@ public static void CloseConnection([NotNull] this DatabaseFacade databaseFacade)
/// Closes the underlying <see cref="DbConnection" />.
/// </summary>
/// <param name="databaseFacade"> The <see cref="DatabaseFacade" /> for the context. </param>
/// <param name="cancellationToken"> A <see cref="CancellationToken" /> to observe while waiting for the task to complete. </param>
/// <returns> A task that represents the asynchronous operation. </returns>
public static Task CloseConnectionAsync([NotNull] this DatabaseFacade databaseFacade, CancellationToken cancellationToken = default)
=> GetFacadeDependencies(databaseFacade).RelationalConnection.CloseAsync(cancellationToken);
public static Task CloseConnectionAsync([NotNull] this DatabaseFacade databaseFacade)
=> GetFacadeDependencies(databaseFacade).RelationalConnection.CloseAsync();

/// <summary>
/// Starts a new transaction with a given <see cref="IsolationLevel" />.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ public virtual async Task ExecuteNonQueryAsync(
}
finally
{
await connection.CloseAsync(cancellationToken);
await connection.CloseAsync();
}
}
finally
Expand Down
5 changes: 1 addition & 4 deletions src/EFCore.Relational/Storage/IRelationalConnection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,11 @@ public interface IRelationalConnection : IRelationalTransactionManager, IDisposa
/// <summary>
/// Closes the connection to the database.
/// </summary>
/// <param name="cancellationToken">
/// A <see cref="CancellationToken" /> to observe while waiting for the task to complete.
/// </param>
/// <returns>
/// A task that represents the asynchronous operation, with a value of true if the connection
/// was actually closed.
/// </returns>
Task<bool> CloseAsync(CancellationToken cancellationToken = default);
Task<bool> CloseAsync();

/// <summary>
/// Gets a value indicating whether the multiple active result sets feature is enabled.
Expand Down
11 changes: 5 additions & 6 deletions src/EFCore.Relational/Storage/RelationalCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -133,12 +133,11 @@ private static void CleanupCommand(

private static async Task CleanupCommandAsync(
DbCommand command,
IRelationalConnection connection,
CancellationToken cancellationToken = default)
IRelationalConnection connection)
{
command.Parameters.Clear();
await command.DisposeAsyncIfAvailable();
await connection.CloseAsync(cancellationToken);
await connection.CloseAsync();
}

/// <summary>
Expand Down Expand Up @@ -214,7 +213,7 @@ await logger.CommandErrorAsync(
}
finally
{
await CleanupCommandAsync(command, connection, cancellationToken);
await CleanupCommandAsync(command, connection);
}
}

Expand Down Expand Up @@ -347,7 +346,7 @@ await logger.CommandErrorAsync(
}
finally
{
await CleanupCommandAsync(command, connection, cancellationToken);
await CleanupCommandAsync(command, connection);
}
}

Expand Down Expand Up @@ -510,7 +509,7 @@ await logger.CommandErrorAsync(
{
if (!readerOpen)
{
await CleanupCommandAsync(command, connection, cancellationToken);
await CleanupCommandAsync(command, connection);
}
}
}
Expand Down
14 changes: 4 additions & 10 deletions src/EFCore.Relational/Storage/RelationalConnection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -655,14 +655,11 @@ public virtual bool Close()
/// <summary>
/// Closes the connection to the database.
/// </summary>
/// <param name="cancellationToken">
/// A <see cref="CancellationToken" /> to observe while waiting for the task to complete.
/// </param>
/// <returns>
/// A task that represents the asynchronous operation, with a value of true if the connection
/// was actually closed.
/// </returns>
public virtual async Task<bool> CloseAsync(CancellationToken cancellationToken = default)
public virtual async Task<bool> CloseAsync()
{
var wasClosed = false;

Expand All @@ -681,8 +678,7 @@ public virtual async Task<bool> CloseAsync(CancellationToken cancellationToken =

var interceptionResult = await Dependencies.ConnectionLogger.ConnectionClosingAsync(
this,
startTime,
cancellationToken);
startTime);

try
{
Expand All @@ -696,8 +692,7 @@ public virtual async Task<bool> CloseAsync(CancellationToken cancellationToken =
await Dependencies.ConnectionLogger.ConnectionClosedAsync(
this,
startTime,
stopwatch.Elapsed,
cancellationToken);
stopwatch.Elapsed);
}
catch (Exception e)
{
Expand All @@ -706,8 +701,7 @@ await Dependencies.ConnectionLogger.ConnectionErrorAsync(
e,
startTime,
stopwatch.Elapsed,
false,
cancellationToken);
false);

throw;
}
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore.Relational/Storage/RelationalTransaction.cs
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ protected virtual async Task ClearTransactionAsync(CancellationToken cancellatio
{
_connectionClosed = true;

await Connection.CloseAsync(cancellationToken);
await Connection.CloseAsync();
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/EFCore.Relational/Update/Internal/BatchExecutor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ private async Task<int> ExecuteAsync(
}
else
{
await connection.CloseAsync(cancellationToken);
await connection.CloseAsync();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -346,8 +346,7 @@ public virtual Task ConnectionOpenedAsync(
public virtual Task<InterceptionResult?> ConnectionClosingAsync(
DbConnection connection,
ConnectionEventData eventData,
InterceptionResult? result,
CancellationToken cancellationToken = default)
InterceptionResult? result)
{
Assert.True(eventData.IsAsync);
AsyncCalled = true;
Expand All @@ -367,8 +366,7 @@ public virtual void ConnectionClosed(

public virtual Task ConnectionClosedAsync(
DbConnection connection,
ConnectionEndEventData eventData,
CancellationToken cancellationToken = default)
ConnectionEndEventData eventData)
{
Assert.True(eventData.IsAsync);
AsyncCalled = true;
Expand Down
4 changes: 1 addition & 3 deletions test/EFCore.Relational.Tests/RelationalEventIdTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -145,11 +145,9 @@ private class FakeRelationalConnection : IRelationalConnection
public DbContext Context => null;
public Guid ConnectionId => Guid.NewGuid();
public int? CommandTimeout { get; set; }
public Task<bool> CloseAsync(CancellationToken cancellationToken = default) => throw new NotImplementedException();
public Task<bool> CloseAsync() => throw new NotImplementedException();
public bool IsMultipleActiveResultSetsEnabled => throw new NotImplementedException();
public IDbContextTransaction CurrentTransaction => throw new NotImplementedException();
public Transaction EnlistedTransaction { get; }
public void EnlistTransaction(Transaction transaction) => throw new NotImplementedException();
public SemaphoreSlim Semaphore => throw new NotImplementedException();
public IDbContextTransaction BeginTransaction(System.Data.IsolationLevel isolationLevel) => throw new NotImplementedException();
public IDbContextTransaction BeginTransaction() => throw new NotImplementedException();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -370,8 +370,6 @@ public void RollbackTransaction()
}

public IDbContextTransaction CurrentTransaction => throw new NotImplementedException();
public Transaction EnlistedTransaction { get; }
public void EnlistTransaction(Transaction transaction) => throw new NotImplementedException();
public SemaphoreSlim Semaphore { get; }

public string ConnectionString { get; }
Expand All @@ -386,7 +384,7 @@ public Task<bool> OpenAsync(CancellationToken cancellationToken, bool errorsExpe

public bool Close() => true;

public Task<bool> CloseAsync(CancellationToken cancellationToken = default) => Task.FromResult<bool>(true);
public Task<bool> CloseAsync() => Task.FromResult<bool>(true);

public bool IsMultipleActiveResultSetsEnabled { get; }
public IDbContextTransaction BeginTransaction(System.Data.IsolationLevel isolationLevel) => throw new NotImplementedException();
Expand Down
16 changes: 15 additions & 1 deletion test/EFCore.Tests/ApiConsistencyTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,19 @@ from parameter in method.GetParameters()
[ConditionalFact]
public virtual void Async_methods_should_have_overload_with_cancellation_token_and_end_with_async_suffix()
{
var withoutCancellationToken = new HashSet<string>
{
"RelationalDatabaseFacadeExtensions.CloseConnectionAsync",
"IRelationalConnection.CloseAsync",
"RelationalConnection.CloseAsync",
"DbConnectionInterceptor.ConnectionClosingAsync",
"DbConnectionInterceptor.ConnectionClosedAsync",
"IDbConnectionInterceptor.ConnectionClosingAsync",
"IDbConnectionInterceptor.ConnectionClosedAsync",
"RelationalLoggerExtensions.ConnectionClosingAsync",
"RelationalLoggerExtensions.ConnectionClosedAsync"
};

var asyncMethods
= (from type in GetAllTypes(TargetAssembly.GetTypes())
where type.GetTypeInfo().IsVisible
Expand All @@ -194,7 +207,8 @@ where method.GetParameters().Any(pi => pi.ParameterType == typeof(CancellationTo

var asyncMethodsWithoutToken
= (from method in asyncMethods
where method.GetParameters().All(pi => pi.ParameterType != typeof(CancellationToken))
where !withoutCancellationToken.Contains(method.DeclaringType.Name + "." + method.Name)
&& method.GetParameters().All(pi => pi.ParameterType != typeof(CancellationToken))
select method).ToList();

var missingOverloads
Expand Down

0 comments on commit 32ff2d1

Please sign in to comment.