Skip to content

Commit

Permalink
Allow execution strategy bypass
Browse files Browse the repository at this point in the history
Do not throw an exception on the first execution of an execution
strategy if a transaction is pending when MaxRetryCount is zero.
Fixes #24922
  • Loading branch information
martincostello authored Jun 9, 2021
1 parent 4a67481 commit eb2cc88
Show file tree
Hide file tree
Showing 2 changed files with 102 additions and 5 deletions.
11 changes: 6 additions & 5 deletions src/EFCore/Storage/ExecutionStrategy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ protected static bool Suspended
/// Indicates whether this <see cref="IExecutionStrategy" /> might retry the execution after a failure.
/// </summary>
public virtual bool RetriesOnFailure
=> !Suspended;
=> !Suspended && MaxRetryCount > 0;

/// <summary>
/// Executes the specified operation and returns the result.
Expand Down Expand Up @@ -325,10 +325,11 @@ private async Task<ExecutionResult<TResult>> ExecuteImplementationAsync<TState,
/// </summary>
protected virtual void OnFirstExecution()
{
if (Dependencies.CurrentContext.Context.Database.CurrentTransaction is not null
|| Dependencies.CurrentContext.Context.Database.GetEnlistedTransaction() is not null
|| (((IDatabaseFacadeDependenciesAccessor)Dependencies.CurrentContext.Context.Database).Dependencies.TransactionManager as
ITransactionEnlistmentManager)?.CurrentAmbientTransaction is not null)
if (RetriesOnFailure &&
(Dependencies.CurrentContext.Context.Database.CurrentTransaction is not null
|| Dependencies.CurrentContext.Context.Database.GetEnlistedTransaction() is not null
|| (((IDatabaseFacadeDependenciesAccessor)Dependencies.CurrentContext.Context.Database).Dependencies.TransactionManager as
ITransactionEnlistmentManager)?.CurrentAmbientTransaction is not null))
{
throw new InvalidOperationException(
CoreStrings.ExecutionStrategyExistingTransaction(
Expand Down
96 changes: 96 additions & 0 deletions test/EFCore.Tests/Storage/ExecutionStrategyTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,54 @@ private void Execute_does_not_throw_when_invoked_twice(Action<ExecutionStrategy,
}
}

[ConditionalFact]
public void Execute_Action_does_not_throw_for_an_existing_transaction_if_RetryOnFailure_disabled()
{
Execute_does_not_throw_for_an_existing_transaction_if_RetryOnFailure_disabled((e, f) => e.Execute(() => f()));
}

[ConditionalFact]
public void Execute_Func_does_not_throw_for_an_existing_transaction_if_RetryOnFailure_disabled()
{
Execute_does_not_throw_for_an_existing_transaction_if_RetryOnFailure_disabled((e, f) => e.Execute(f));
}

private void Execute_does_not_throw_for_an_existing_transaction_if_RetryOnFailure_disabled(
Action<ExecutionStrategy, Func<int>> execute)
{
using var context1 = CreateContext();
using var context2 = CreateContext();

var mockExecutionStrategy1 = new TestExecutionStrategy(context1, retryCount: 0);
var mockExecutionStrategy2 = new TestExecutionStrategy(context2, retryCount: 0);

using var tran1 = context1.Database.BeginTransaction();
using var tran2 = context2.Database.BeginTransaction();

var executed1 = false;
var executed2 = false;

execute(
mockExecutionStrategy1, () =>
{
executed1 = true;
return 0;
});

execute(
mockExecutionStrategy2, () =>
{
executed2 = true;
return 0;
});

tran1.Commit();
tran2.Commit();

Assert.True(executed1);
Assert.True(executed2);
}

[ConditionalFact]
public void Execute_Action_doesnt_retry_if_successful()
{
Expand Down Expand Up @@ -475,6 +523,54 @@ await executeAsync(
}
}

[ConditionalFact]
public async Task ExecuteAsync_Action_does_not_throw_for_an_existing_transaction_if_RetryOnFailure_disabled()
{
await ExecuteAsync_does_not_throw_for_an_existing_transaction_if_RetryOnFailure_disabled((e, f) => e.ExecuteAsync(() => (Task)f(CancellationToken.None)));
}

[ConditionalFact]
public async Task ExecuteAsync_Func_does_not_throw_for_an_existing_transaction_if_RetryOnFailure_disabled()
{
await ExecuteAsync_does_not_throw_for_an_existing_transaction_if_RetryOnFailure_disabled((e, f) => e.ExecuteAsync(f, CancellationToken.None));
}

private async Task ExecuteAsync_does_not_throw_for_an_existing_transaction_if_RetryOnFailure_disabled(
Func<ExecutionStrategy, Func<CancellationToken, Task<int>>, Task> executeAsync)
{
await using var context1 = CreateContext();
await using var context2 = CreateContext();

var mockExecutionStrategy1 = new TestExecutionStrategy(context1, retryCount: 0);
var mockExecutionStrategy2 = new TestExecutionStrategy(context2, retryCount: 0);

await using var tran1 = context1.Database.BeginTransaction();
await using var tran2 = context2.Database.BeginTransaction();

var executed1 = false;
var executed2 = false;

await executeAsync(
mockExecutionStrategy1, ct =>
{
executed1 = true;
return Task.FromResult(0);
});

await executeAsync(
mockExecutionStrategy2, ct =>
{
executed2 = true;
return Task.FromResult(0);
});

await tran1.CommitAsync();
await tran2.CommitAsync();

Assert.True(executed1);
Assert.True(executed2);
}

[ConditionalFact]
public Task ExecuteAsync_Action_doesnt_retry_if_successful()
{
Expand Down

0 comments on commit eb2cc88

Please sign in to comment.