From e15f307b7e6bcc7a2e60aa1409806cedbeb0f697 Mon Sep 17 00:00:00 2001 From: martincostello Date: Sat, 5 Jun 2021 15:17:54 +0100 Subject: [PATCH] Allow execution strategy bypass Do not throw an exception on the first execution of an execution strategy if a transaction is pending when MaxRetryCount is zero. Addresses #24922. --- src/EFCore/Storage/ExecutionStrategy.cs | 31 +++--- .../Storage/ExecutionStrategyTest.cs | 96 +++++++++++++++++++ 2 files changed, 113 insertions(+), 14 deletions(-) diff --git a/src/EFCore/Storage/ExecutionStrategy.cs b/src/EFCore/Storage/ExecutionStrategy.cs index 7e70d2aa9f0..6177b4f3ef8 100644 --- a/src/EFCore/Storage/ExecutionStrategy.cs +++ b/src/EFCore/Storage/ExecutionStrategy.cs @@ -126,7 +126,7 @@ protected static bool Suspended /// Indicates whether this might retry the execution after a failure. /// public virtual bool RetriesOnFailure - => !Suspended; + => !Suspended && MaxRetryCount > 0; /// /// Executes the specified operation and returns the result. @@ -325,20 +325,23 @@ private async Task> ExecuteImplementationAsync 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) { - throw new InvalidOperationException( - CoreStrings.ExecutionStrategyExistingTransaction( - GetType().Name, - nameof(DbContext) - + "." - + nameof(DbContext.Database) - + "." - + nameof(DatabaseFacade.CreateExecutionStrategy) - + "()")); + 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) + { + throw new InvalidOperationException( + CoreStrings.ExecutionStrategyExistingTransaction( + GetType().Name, + nameof(DbContext) + + "." + + nameof(DbContext.Database) + + "." + + nameof(DatabaseFacade.CreateExecutionStrategy) + + "()")); + } } ExceptionsEncountered.Clear(); diff --git a/test/EFCore.Tests/Storage/ExecutionStrategyTest.cs b/test/EFCore.Tests/Storage/ExecutionStrategyTest.cs index 66cff72708d..5980f3beb32 100644 --- a/test/EFCore.Tests/Storage/ExecutionStrategyTest.cs +++ b/test/EFCore.Tests/Storage/ExecutionStrategyTest.cs @@ -196,6 +196,54 @@ private void Execute_does_not_throw_when_invoked_twice(Action 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> 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() { @@ -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>, 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() {