From 7e7913a71442b7b137bfb73a7e2b44a35d50d099 Mon Sep 17 00:00:00 2001 From: AndriySvyryd Date: Wed, 23 Aug 2017 12:33:26 -0700 Subject: [PATCH] Add support for System.Transactions Fixes #5595 --- .../Internal/InMemoryTransactionManager.cs | 5 + ...Core.Relational.Specification.Tests.csproj | 4 + .../TransactionTestBase.cs | 393 ++++++++++++- .../Internal/MigrationCommandExecutor.cs | 99 ++-- .../Properties/RelationalStrings.Designer.cs | 14 +- .../Properties/RelationalStrings.resx | 64 ++- .../Storage/RelationalConnection.cs | 340 +++++++++--- .../Update/Internal/BatchExecutor.cs | 5 + .../Storage/Internal/SqlServerConnection.cs | 5 + src/EFCore/Infrastructure/DatabaseFacade.cs | 14 + src/EFCore/Storage/ExecutionStrategy.cs | 5 +- .../Storage/IDbContextTransactionManager.cs | 14 + src/EFCore/breakingchanges.netcore.json | 12 +- .../EFCore.Relational.Tests.csproj | 4 + .../RelationalDatabaseFacadeExtensionsTest.cs | 5 + .../RelationalEventIdTest.cs | 4 + .../EFCore.SqlServer.FunctionalTests.csproj | 4 + .../SqlServerDatabaseCreatorTest.cs | 522 +++++++----------- .../TestUtilities/TestSqlServerConnection.cs | 5 + .../TransactionSqlServerTest.cs | 4 + .../EFCore.Sqlite.FunctionalTests.csproj | 4 + .../Query/BadDataSqliteTest.cs | 2 + .../EFCore.Sqlite.Tests.csproj | 4 + test/EFCore.Tests/DatabaseFacadeTest.cs | 2 + test/EFCore.Tests/EFCore.Tests.csproj | 4 + .../Storage/ExecutionStrategyTest.cs | 247 ++++++--- .../TestInMemoryTransactionManager.cs | 19 +- 27 files changed, 1210 insertions(+), 594 deletions(-) diff --git a/src/EFCore.InMemory/Storage/Internal/InMemoryTransactionManager.cs b/src/EFCore.InMemory/Storage/Internal/InMemoryTransactionManager.cs index 98f486a7a89..5f393d773b4 100644 --- a/src/EFCore.InMemory/Storage/Internal/InMemoryTransactionManager.cs +++ b/src/EFCore.InMemory/Storage/Internal/InMemoryTransactionManager.cs @@ -3,6 +3,7 @@ using System.Threading; using System.Threading.Tasks; +using System.Transactions; using JetBrains.Annotations; using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.EntityFrameworkCore.Internal; @@ -44,6 +45,10 @@ public virtual IDbContextTransaction BeginTransaction() public virtual void RollbackTransaction() => _logger.TransactionIgnoredWarning(); public virtual IDbContextTransaction CurrentTransaction => null; + public virtual Transaction EnlistedTransaction => null; + public virtual void EnlistTransaction(Transaction transaction) + { + } public virtual void ResetState() { diff --git a/src/EFCore.Relational.Specification.Tests/EFCore.Relational.Specification.Tests.csproj b/src/EFCore.Relational.Specification.Tests/EFCore.Relational.Specification.Tests.csproj index d855371a046..96eb5c35e94 100644 --- a/src/EFCore.Relational.Specification.Tests/EFCore.Relational.Specification.Tests.csproj +++ b/src/EFCore.Relational.Specification.Tests/EFCore.Relational.Specification.Tests.csproj @@ -17,6 +17,10 @@ + + + + diff --git a/src/EFCore.Relational.Specification.Tests/TransactionTestBase.cs b/src/EFCore.Relational.Specification.Tests/TransactionTestBase.cs index 6749299d48a..d653bcdc98a 100644 --- a/src/EFCore.Relational.Specification.Tests/TransactionTestBase.cs +++ b/src/EFCore.Relational.Specification.Tests/TransactionTestBase.cs @@ -6,6 +6,7 @@ using System.Data; using System.Linq; using System.Threading.Tasks; +using System.Transactions; using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.EntityFrameworkCore.Internal; using Microsoft.EntityFrameworkCore.Storage; @@ -108,6 +109,134 @@ public virtual async Task SaveChangesAsync_implicitly_starts_transaction() AssertStoreInitialState(); } + [Theory] + [InlineData(true)] + [InlineData(false)] + public virtual void SaveChanges_uses_enlisted_transaction(bool autoTransactionsEnabled) + { + if (!AmbientTransactionsSupported) + { + return; + } + + using (var transaction = new CommittableTransaction()) + { + using (var context = CreateContext()) + { + context.Database.EnlistTransaction(transaction); + context.Database.AutoTransactionsEnabled = autoTransactionsEnabled; + + context.Add(new TransactionCustomer { Id = 77, Name = "Bobble" }); + context.Entry(context.Set().Last()).State = EntityState.Added; + + Assert.Throws(() => context.SaveChanges()); + } + + using (var context = CreateContext()) + { + Assert.NotNull(context.Set().Find(77)); + } + } + + AssertStoreInitialState(); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public virtual async Task SaveChangesAsync_uses_enlisted_transaction(bool autoTransactionsEnabled) + { + if (!AmbientTransactionsSupported) + { + return; + } + + using (var transaction = new CommittableTransaction()) + { + using (var context = CreateContext()) + { + context.Database.EnlistTransaction(transaction); + context.Database.AutoTransactionsEnabled = autoTransactionsEnabled; + + context.Add(new TransactionCustomer { Id = 77, Name = "Bobble" }); + context.Entry(context.Set().Last()).State = EntityState.Added; + + try + { + await context.SaveChangesAsync(); + } + catch (DbUpdateException) + { + } + } + + using (var context = CreateContext()) + { + Assert.NotNull(await context.Set().FindAsync( 77 )); + } + } + + AssertStoreInitialState(); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public virtual void SaveChanges_uses_ambient_transaction(bool autoTransactionsEnabled) + { + if (!AmbientTransactionsSupported) + { + return; + } + + using (new TransactionScope()) + { + using (var context = CreateContext()) + { + context.Database.AutoTransactionsEnabled = autoTransactionsEnabled; + + context.Add(new TransactionCustomer { Id = 77, Name = "Bobble" }); + context.Entry(context.Set().Last()).State = EntityState.Added; + + Assert.Throws(() => context.SaveChanges()); + } + } + + AssertStoreInitialState(); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public virtual async Task SaveChangesAsync_uses_ambient_transaction(bool autoTransactionsEnabled) + { + if (!AmbientTransactionsSupported) + { + return; + } + + using (new TransactionScope(TransactionScopeAsyncFlowOption.Enabled)) + { + using (var context = CreateContext()) + { + context.Database.AutoTransactionsEnabled = autoTransactionsEnabled; + + context.Add(new TransactionCustomer { Id = 77, Name = "Bobble" }); + context.Entry(context.Set().Last()).State = EntityState.Added; + + try + { + await context.SaveChangesAsync(); + } + catch (DbUpdateException) + { + } + } + } + + AssertStoreInitialState(); + } + [Fact] public virtual void SaveChanges_does_not_close_connection_opened_by_user() { @@ -428,7 +557,8 @@ public virtual void Query_uses_explicit_transaction(bool autoTransaction) if (DirtyReadsOccur) { - using (innerContext.Database.BeginTransaction(IsolationLevel.ReadUncommitted)) + // ReSharper disable once RedundantNameQualifier + using (innerContext.Database.BeginTransaction(System.Data.IsolationLevel.ReadUncommitted)) { Assert.Equal(Customers.Count - 1, innerContext.Set().Count()); } @@ -436,7 +566,8 @@ public virtual void Query_uses_explicit_transaction(bool autoTransaction) if (SnapshotSupported) { - using (innerContext.Database.BeginTransaction(IsolationLevel.Snapshot)) + // ReSharper disable once RedundantNameQualifier + using (innerContext.Database.BeginTransaction(System.Data.IsolationLevel.Snapshot)) { Assert.Equal(Customers, innerContext.Set().OrderBy(c => c.Id).ToList()); } @@ -474,7 +605,8 @@ public virtual async Task QueryAsync_uses_explicit_transaction(bool autoTransact if (DirtyReadsOccur) { - using (await innerContext.Database.BeginTransactionAsync(IsolationLevel.ReadUncommitted)) + // ReSharper disable once RedundantNameQualifier + using (await innerContext.Database.BeginTransactionAsync(System.Data.IsolationLevel.ReadUncommitted)) { Assert.Equal(Customers.Count - 1, await innerContext.Set().CountAsync()); } @@ -482,7 +614,8 @@ public virtual async Task QueryAsync_uses_explicit_transaction(bool autoTransact if (SnapshotSupported) { - using (await innerContext.Database.BeginTransactionAsync(IsolationLevel.Snapshot)) + // ReSharper disable once RedundantNameQualifier + using (await innerContext.Database.BeginTransactionAsync(System.Data.IsolationLevel.Snapshot)) { Assert.Equal(Customers, await innerContext.Set().OrderBy(c => c.Id).ToListAsync()); } @@ -521,17 +654,13 @@ public virtual async Task Can_use_open_connection_with_started_transaction(bool AssertStoreInitialState(); } - [Theory] - [InlineData(true)] - [InlineData(false)] - public virtual void UseTransaction_throws_if_mismatched_connection(bool autoTransaction) + [Fact] + public virtual void UseTransaction_throws_if_mismatched_connection() { using (var transaction = TestStore.BeginTransaction()) { using (var context = CreateContextWithConnectionString()) { - context.Database.AutoTransactionsEnabled = autoTransaction; - var ex = Assert.Throws( () => context.Database.UseTransaction(transaction)); @@ -540,17 +669,13 @@ public virtual void UseTransaction_throws_if_mismatched_connection(bool autoTran } } - [Theory] - [InlineData(true)] - [InlineData(false)] - public virtual void UseTransaction_throws_if_another_transaction_started(bool autoTransaction) + [Fact] + public virtual void UseTransaction_throws_if_another_transaction_started() { using (var transaction = TestStore.BeginTransaction()) { using (var context = CreateContextWithConnectionString()) { - context.Database.AutoTransactionsEnabled = autoTransaction; - using (context.Database.BeginTransaction()) { var ex = Assert.Throws( @@ -562,17 +687,30 @@ public virtual void UseTransaction_throws_if_another_transaction_started(bool au } } - [Theory] - [InlineData(true)] - [InlineData(false)] - public virtual void UseTransaction_will_not_dispose_external_transaction(bool autoTransaction) + [Fact] + public virtual void UseTransaction_throws_if_ambient_transaction_started() + { + using (new TransactionScope()) + { + using (var transaction = TestStore.BeginTransaction()) + { + using (var context = CreateContextWithConnectionString()) + { + var ex = Assert.Throws( + () => context.Database.UseTransaction(transaction)); + Assert.Equal(RelationalStrings.ConflictingAmbientTransaction, ex.Message); + } + } + } + } + + [Fact] + public virtual void UseTransaction_will_not_dispose_external_transaction() { using (var transaction = TestStore.BeginTransaction()) { using (var context = CreateContext()) { - context.Database.AutoTransactionsEnabled = autoTransaction; - context.Database.UseTransaction(transaction); context.Database.GetService().Dispose(); @@ -582,6 +720,215 @@ public virtual void UseTransaction_will_not_dispose_external_transaction(bool au } } + [Fact] + public virtual void UseTransaction_throws_if_enlisted_in_transaction() + { + if (!AmbientTransactionsSupported) + { + return; + } + using (var t = new CommittableTransaction()) + { + using (var transaction = TestStore.BeginTransaction()) + { + using (var context = CreateContextWithConnectionString()) + { + context.Database.OpenConnection(); + + context.Database.EnlistTransaction(t); + + var ex = Assert.Throws( + () => context.Database.UseTransaction(transaction)); + Assert.Equal(RelationalStrings.ConflictingEnlistedTransaction, ex.Message); + } + } + } + } + + [Fact] + public virtual void BeginTransaction_throws_if_another_transaction_started() + { + using (var context = CreateContextWithConnectionString()) + { + using (context.Database.BeginTransaction()) + { + var ex = Assert.Throws( + () => context.Database.BeginTransaction()); + Assert.Equal(RelationalStrings.TransactionAlreadyStarted, ex.Message); + } + } + } + + [Fact] + public virtual void BeginTransaction_throws_if_ambient_transaction_started() + { + if (!AmbientTransactionsSupported) + { + return; + } + + using (new TransactionScope()) + { + using (var context = CreateContextWithConnectionString()) + { + var ex = Assert.Throws( + () => context.Database.BeginTransaction()); + Assert.Equal(RelationalStrings.ConflictingAmbientTransaction, ex.Message); + } + } + } + + [Fact] + public virtual void BeginTransaction_throws_if_enlisted_in_transaction() + { + if (!AmbientTransactionsSupported) + { + return; + } + + using (var transaction = new CommittableTransaction()) + { + using (var context = CreateContextWithConnectionString()) + { + context.Database.OpenConnection(); + + context.Database.EnlistTransaction(transaction); + + var ex = Assert.Throws( + () => context.Database.BeginTransaction()); + Assert.Equal(RelationalStrings.ConflictingEnlistedTransaction, ex.Message); + } + } + } + + [Fact] + public virtual void BeginTransaction_can_be_used_after_ambient_transaction_ended() + { + if (!AmbientTransactionsSupported) + { + return; + } + + using (var context = CreateContextWithConnectionString()) + { + using (new TransactionScope()) + { + context.Database.OpenConnection(); + } + + using (context.Database.BeginTransaction()) + { + } + } + } + + [Fact] + public virtual void BeginTransaction_can_be_used_after_enlisted_transaction_ended() + { + if (!AmbientTransactionsSupported) + { + return; + } + + using (var context = CreateContextWithConnectionString()) + { + using (var transaction = new CommittableTransaction()) + { + context.Database.OpenConnection(); + + context.Database.EnlistTransaction(transaction); + } + + using (context.Database.BeginTransaction()) + { + } + } + } + + [Fact] + public virtual void BeginTransaction_can_be_used_after_another_transaction_connection_closed() + { + using (var context = CreateContextWithConnectionString()) + { + using (context.Database.BeginTransaction()) + { + context.Database.CloseConnection(); + using (context.Database.BeginTransaction()) + { + } + } + } + } + + [Fact] + public virtual void BeginTransaction_can_be_used_after_enlisted_transaction_connection_closed() + { + if (!AmbientTransactionsSupported) + { + return; + } + + using (var context = CreateContextWithConnectionString()) + { + using (var transaction = new CommittableTransaction()) + { + context.Database.OpenConnection(); + + context.Database.EnlistTransaction(transaction); + + context.Database.CloseConnection(); + + using (context.Database.BeginTransaction()) + { + } + } + } + } + + [Fact] + public virtual void EnlistTransaction_throws_if_another_transaction_started() + { + if (!AmbientTransactionsSupported) + { + return; + } + + using (var transaction = new CommittableTransaction()) + { + using (var context = CreateContextWithConnectionString()) + { + using (context.Database.BeginTransaction()) + { + Assert.Throws( + () => context.Database.EnlistTransaction(transaction)); + } + } + } + } + + [Fact] + public virtual void EnlistTransaction_throws_if_ambient_transaction_started() + { + if (!AmbientTransactionsSupported) + { + return; + } + + using (new TransactionScope()) + { + using (var transaction = new CommittableTransaction()) + { + using (var context = CreateContextWithConnectionString()) + { + context.Database.OpenConnection(); + + Assert.Throws( + () => context.Database.EnlistTransaction(transaction)); + } + } + } + } + protected virtual void AssertStoreInitialState() { using (var context = CreateContext()) @@ -594,6 +941,8 @@ protected virtual void AssertStoreInitialState() protected abstract bool SnapshotSupported { get; } + protected virtual bool AmbientTransactionsSupported => false; + protected virtual bool DirtyReadsOccur => true; protected DbContext CreateContext() => Fixture.CreateContext(); diff --git a/src/EFCore.Relational/Migrations/Internal/MigrationCommandExecutor.cs b/src/EFCore.Relational/Migrations/Internal/MigrationCommandExecutor.cs index 177e76f8631..e18b0dd94e7 100644 --- a/src/EFCore.Relational/Migrations/Internal/MigrationCommandExecutor.cs +++ b/src/EFCore.Relational/Migrations/Internal/MigrationCommandExecutor.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; +using System.Transactions; using Microsoft.EntityFrameworkCore.Storage; using Microsoft.EntityFrameworkCore.Utilities; @@ -26,44 +27,47 @@ public class MigrationCommandExecutor : IMigrationCommandExecutor Check.NotNull(migrationCommands, nameof(migrationCommands)); Check.NotNull(connection, nameof(connection)); - connection.Open(); - - try + using (new TransactionScope(TransactionScopeOption.Suppress)) { - IDbContextTransaction transaction = null; + connection.Open(); try { - foreach (var command in migrationCommands) + IDbContextTransaction transaction = null; + + try { - if (transaction == null - && !command.TransactionSuppressed) + foreach (var command in migrationCommands) { - transaction = connection.BeginTransaction(); - } + if (transaction == null + && !command.TransactionSuppressed) + { + transaction = connection.BeginTransaction(); + } - if (transaction != null - && command.TransactionSuppressed) - { - transaction.Commit(); - transaction.Dispose(); - transaction = null; + if (transaction != null + && command.TransactionSuppressed) + { + transaction.Commit(); + transaction.Dispose(); + transaction = null; + } + + command.ExecuteNonQuery(connection); } - command.ExecuteNonQuery(connection); + transaction?.Commit(); + } + finally + { + transaction?.Dispose(); } - - transaction?.Commit(); } finally { - transaction?.Dispose(); + connection.Close(); } } - finally - { - connection.Close(); - } } /// @@ -78,44 +82,47 @@ public class MigrationCommandExecutor : IMigrationCommandExecutor Check.NotNull(migrationCommands, nameof(migrationCommands)); Check.NotNull(connection, nameof(connection)); - await connection.OpenAsync(cancellationToken); - - try + using (new TransactionScope(TransactionScopeOption.Suppress, TransactionScopeAsyncFlowOption.Enabled)) { - IDbContextTransaction transaction = null; + await connection.OpenAsync(cancellationToken); try { - foreach (var command in migrationCommands) + IDbContextTransaction transaction = null; + + try { - if (transaction == null - && !command.TransactionSuppressed) + foreach (var command in migrationCommands) { - transaction = await connection.BeginTransactionAsync(cancellationToken); - } + if (transaction == null + && !command.TransactionSuppressed) + { + transaction = await connection.BeginTransactionAsync(cancellationToken); + } - if (transaction != null - && command.TransactionSuppressed) - { - transaction.Commit(); - transaction.Dispose(); - transaction = null; + if (transaction != null + && command.TransactionSuppressed) + { + transaction.Commit(); + transaction.Dispose(); + transaction = null; + } + + await command.ExecuteNonQueryAsync(connection, cancellationToken: cancellationToken); } - await command.ExecuteNonQueryAsync(connection, cancellationToken: cancellationToken); + transaction?.Commit(); + } + finally + { + transaction?.Dispose(); } - - transaction?.Commit(); } finally { - transaction?.Dispose(); + connection.Close(); } } - finally - { - connection.Close(); - } } } } diff --git a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs index 014447c5fc5..19d036ce91d 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs +++ b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs @@ -319,7 +319,7 @@ public static string DiscriminatorPropertyNotFound([CanBeNull] object property, property, entityType); /// - /// An ambient transaction has been detected. Entity Framework Core does not support ambient transactions. See http://go.microsoft.com/fwlink/?LinkId=800142 + /// An ambient transaction has been detected. The current provider does not support ambient transactions. See http://go.microsoft.com/fwlink/?LinkId=800142 /// public static readonly EventDefinition LogAmbientTransaction = new EventDefinition( @@ -872,6 +872,18 @@ public static string DbFunctionExpressionIsNotMethodCall([CanBeNull] object expr GetString("DbFunctionExpressionIsNotMethodCall", nameof(expression)), expression); + /// + /// An ambient transaction has been detected. The ambient transaction needs to be completed before beginning a transaction on this connection. + /// + public static string ConflictingAmbientTransaction + => GetString("ConflictingAmbientTransaction"); + + /// + /// The connection is currently enlisted in a transaction. The enlisted transaction needs to be completed before starting a transaction. + /// + public static string ConflictingEnlistedTransaction + => GetString("ConflictingEnlistedTransaction"); + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/EFCore.Relational/Properties/RelationalStrings.resx b/src/EFCore.Relational/Properties/RelationalStrings.resx index 92972220f92..e37cc82e9c4 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.resx +++ b/src/EFCore.Relational/Properties/RelationalStrings.resx @@ -1,17 +1,17 @@  - @@ -233,7 +233,7 @@ Unable to set property '{property}' as a discriminator for entity type '{entityType}' because it is not a property of '{entityType}'. - An ambient transaction has been detected. Entity Framework Core does not support ambient transactions. See http://go.microsoft.com/fwlink/?LinkId=800142 + An ambient transaction has been detected. The current provider does not support ambient transactions. See http://go.microsoft.com/fwlink/?LinkId=800142 Warning RelationalEventId.AmbientTransactionWarning @@ -432,6 +432,12 @@ The DbFunction '{function}' is generic. Generic methods are not supported. - The provided DbFunction expression '{expression}' is invalid. The expression should be a lambda expression containing a single method call to the target static method. Default values can be provided as arguments if required. E.g. () => SomeClass.SomeMethod(null, 0) + The provided DbFunction expression '{expression}' is invalid. The expression should be a lambda expression containing a single method call to the target static method. Default values can be provided as arguments if required. E.g. () => SomeClass.SomeMethod(null, 0) + + + An ambient transaction has been detected. The ambient transaction needs to be completed before beginning a transaction on this connection. + + + The connection is currently enlisted in a transaction. The enlisted transaction needs to be completed before starting a transaction. \ No newline at end of file diff --git a/src/EFCore.Relational/Storage/RelationalConnection.cs b/src/EFCore.Relational/Storage/RelationalConnection.cs index 56710727ba7..e9f6b8722ac 100644 --- a/src/EFCore.Relational/Storage/RelationalConnection.cs +++ b/src/EFCore.Relational/Storage/RelationalConnection.cs @@ -103,6 +103,53 @@ protected RelationalConnection([NotNull] RelationalConnectionDependencies depend /// public virtual IDbContextTransaction CurrentTransaction { get; [param: CanBeNull] protected set; } + /// + /// The currently enlisted transaction. + /// + public virtual Transaction EnlistedTransaction + { + get + { + if (_enlistedTransaction != null) + { + try + { + if (_enlistedTransaction.TransactionInformation.Status != TransactionStatus.Active) + { + _enlistedTransaction = null; + } + } + catch (ObjectDisposedException) + { + _enlistedTransaction = null; + } + } + return _enlistedTransaction; + } + [param: CanBeNull] protected set { _enlistedTransaction = value; } + } + + /// + /// Specifies an existing to be used for database operations. + /// + /// The transaction to be used. + public virtual void EnlistTransaction(Transaction transaction) + { + DbConnection.EnlistTransaction(transaction); + + EnlistedTransaction = transaction; + } + + /// + /// The last ambient transaction used. + /// + public virtual Transaction AmbientTransaction { get; [param: CanBeNull] protected set; } + + /// + /// Indicates whether the store connection supports ambient transactions + /// + protected virtual bool SupportsAmbientTransactions => false; + /// /// Gets the timeout for executing a command against the database. /// @@ -147,13 +194,10 @@ public virtual async Task BeginTransactionAsync(Cancellat [NotNull] public virtual IDbContextTransaction BeginTransaction(IsolationLevel isolationLevel) { - if (CurrentTransaction != null) - { - throw new InvalidOperationException(RelationalStrings.TransactionAlreadyStarted); - } - Open(); + EnsureNoTransactions(); + return BeginTransactionWithNoPreconditions(isolationLevel); } @@ -169,15 +213,30 @@ public virtual IDbContextTransaction BeginTransaction(IsolationLevel isolationLe public virtual async Task BeginTransactionAsync( IsolationLevel isolationLevel, CancellationToken cancellationToken = default(CancellationToken)) + { + await OpenAsync(cancellationToken); + + EnsureNoTransactions(); + + return BeginTransactionWithNoPreconditions(isolationLevel); + } + + private void EnsureNoTransactions() { if (CurrentTransaction != null) { throw new InvalidOperationException(RelationalStrings.TransactionAlreadyStarted); } - await OpenAsync(cancellationToken); + if (Transaction.Current != null) + { + throw new InvalidOperationException(RelationalStrings.ConflictingAmbientTransaction); + } - return BeginTransactionWithNoPreconditions(isolationLevel); + if (EnlistedTransaction != null) + { + throw new InvalidOperationException(RelationalStrings.ConflictingEnlistedTransaction); + } } private IDbContextTransaction BeginTransactionWithNoPreconditions(IsolationLevel isolationLevel) @@ -215,10 +274,7 @@ public virtual IDbContextTransaction UseTransaction(DbTransaction transaction) } else { - if (CurrentTransaction != null) - { - throw new InvalidOperationException(RelationalStrings.TransactionAlreadyStarted); - } + EnsureNoTransactions(); Open(); @@ -273,8 +329,6 @@ public virtual void RollbackTransaction() /// True if the underlying connection was actually opened; false otherwise. public virtual bool Open(bool errorsExpected = false) { - CheckForAmbientTransactions(); - if (DbConnection.State == ConnectionState.Broken) { DbConnection.Close(); @@ -284,50 +338,18 @@ public virtual bool Open(bool errorsExpected = false) if (DbConnection.State != ConnectionState.Open) { - var startTime = DateTimeOffset.UtcNow; - var stopwatch = Stopwatch.StartNew(); - - Dependencies.ConnectionLogger.ConnectionOpening( - this, - startTime, - async: false); - - try - { - DbConnection.Open(); - - wasOpened = true; - - Dependencies.ConnectionLogger.ConnectionOpened( - this, - startTime, - stopwatch.Elapsed, - async: false); - } - catch (Exception e) - { - Dependencies.ConnectionLogger.ConnectionError( - this, - e, - startTime, - stopwatch.Elapsed, - async: false, - logErrorAsDebug: errorsExpected); - - throw; - } - - if (_openedCount == 0) - { - _openedInternally = true; - _openedCount++; - } + OpenDbConnection(errorsExpected); + wasOpened = true; + CurrentTransaction = null; + EnlistedTransaction = null; } else { _openedCount++; } + HandleAmbientTransactions(() => OpenDbConnection(errorsExpected)); + return wasOpened; } @@ -344,8 +366,6 @@ public virtual bool Open(bool errorsExpected = false) /// public virtual async Task OpenAsync(CancellationToken cancellationToken, bool errorsExpected = false) { - CheckForAmbientTransactions(); - if (DbConnection.State == ConnectionState.Broken) { DbConnection.Close(); @@ -355,60 +375,199 @@ public virtual async Task OpenAsync(CancellationToken cancellationToken, b if (DbConnection.State != ConnectionState.Open) { - var startTime = DateTimeOffset.UtcNow; - var stopwatch = Stopwatch.StartNew(); + await OpenDbConnectionAsync(errorsExpected, cancellationToken); + wasOpened = true; + CurrentTransaction = null; + EnlistedTransaction = null; + } + else + { + _openedCount++; + } + + await HandleAmbientTransactionsAsync(() => OpenDbConnectionAsync(errorsExpected, cancellationToken), cancellationToken); + + return wasOpened; + } + + private void OpenDbConnection(bool errorsExpected) + { + var startTime = DateTimeOffset.UtcNow; + var stopwatch = Stopwatch.StartNew(); + + Dependencies.ConnectionLogger.ConnectionOpening( + this, + startTime, + async: false); + + try + { + DbConnection.Open(); - Dependencies.ConnectionLogger.ConnectionOpening( + Dependencies.ConnectionLogger.ConnectionOpened( this, startTime, - async: true); + stopwatch.Elapsed, + async: false); + } + catch (Exception e) + { + Dependencies.ConnectionLogger.ConnectionError( + this, + e, + startTime, + stopwatch.Elapsed, + async: false, + logErrorAsDebug: errorsExpected); - try - { - await DbConnection.OpenAsync(cancellationToken); + throw; + } - wasOpened = true; + if (_openedCount == 0) + { + _openedInternally = true; + _openedCount++; + } + } - Dependencies.ConnectionLogger.ConnectionOpened( - this, - startTime, - stopwatch.Elapsed, - async: true); - } - catch (Exception e) - { - Dependencies.ConnectionLogger.ConnectionError( - this, - e, - startTime, - stopwatch.Elapsed, - async: true, - logErrorAsDebug: errorsExpected); - - throw; - } + private async Task OpenDbConnectionAsync(bool errorsExpected, CancellationToken cancellationToken) + { + var startTime = DateTimeOffset.UtcNow; + var stopwatch = Stopwatch.StartNew(); - if (_openedCount == 0) - { - _openedInternally = true; - _openedCount++; - } + Dependencies.ConnectionLogger.ConnectionOpening( + this, + startTime, + async: true); + + try + { + await DbConnection.OpenAsync(cancellationToken); + + Dependencies.ConnectionLogger.ConnectionOpened( + this, + startTime, + stopwatch.Elapsed, + async: true); } - else + catch (Exception e) { - _openedCount++; + Dependencies.ConnectionLogger.ConnectionError( + this, + e, + startTime, + stopwatch.Elapsed, + async: true, + logErrorAsDebug: errorsExpected); + + throw; } - return wasOpened; + if (_openedCount == 0) + { + _openedInternally = true; + _openedCount++; + } } - // ReSharper disable once MemberCanBeMadeStatic.Local - private void CheckForAmbientTransactions() + /// + /// Ensures the connection is enlisted in the ambient transaction if present. + /// + /// A delegate to open the underlying connection. + protected virtual void HandleAmbientTransactions([NotNull] Action open) + => HandleAmbientTransactions(() => + { + open(); + return Task.CompletedTask; + }); + + /// + /// Ensures the connection is enlisted in the ambient transaction if present. + /// + /// A delegate to open the underlying connection. + /// + /// A to observe while waiting for the task to complete. + /// + /// A task that represents the asynchronous operation. + protected virtual Task HandleAmbientTransactionsAsync( + [NotNull] Func openAsync, CancellationToken cancellationToken = default(CancellationToken)) + => HandleAmbientTransactions(openAsync); + + private Task HandleAmbientTransactions(Func open) { - if (Transaction.Current != null) + var current = Transaction.Current; + if (current != null + && !SupportsAmbientTransactions) { Dependencies.TransactionLogger.AmbientTransactionWarning(this, DateTimeOffset.UtcNow); } + + if (Equals(current, AmbientTransaction)) + { + return Task.CompletedTask; + } + + if (!_openedInternally) + { + // We didn't open the connection so, just try to enlist the connection in the current transaction. + // Note that the connection can already be enlisted in a transaction (since the user opened + // it they could enlist it manually using DbConnection.EnlistTransaction() method). If the + // transaction the connection is enlisted in has not completed (e.g. nested transaction) this call + // will fail (throw). Also "current" can be "null" here which means that the transaction + // used in the previous operation has completed. In this case we should not enlist the connection + // in "null" transaction as the user might have enlisted in a transaction manually between calls by + // calling DbConnection.EnlistTransaction() method. Enlisting with "null" would in this case mean "unenlist" + // and would cause an exception (see above). Had the user not enlisted in a transaction between the calls + // enlisting with null would be a no-op - so again no reason to do it. + if (current != null) + { + DbConnection.EnlistTransaction(current); + } + } + else if (_openedCount > 1) + { + // We opened the connection. In addition we are here because there are multiple + // active requests going on (read: enumerators that has not been disposed yet) + // using the same connection. (If there is only one active request e.g. like SaveChanges + // or single enumerator there is no need for any specific transaction handling - either + // we use the implicit ambient transaction (Transaction.Current) if one exists or we + // will create our own local transaction. Also if there is only one active request + // the user could not enlist it in a transaction using EntityConnection.EnlistTransaction() + // because we opened the connection). + // If there are multiple active requests the user might have "played" with transactions + // after the first transaction. This code tries to deal with this kind of changes. + + if (AmbientTransaction == null) + { + // Two cases here: + // - the previous operation was not run inside a transaction created by the user while this one is - just + // enlist the connection in the transaction + // - the previous operation ran withing explicit transaction started with EntityConnection.EnlistTransaction() + // method - try enlisting the connection in the transaction. This may fail however if the transactions + // are nested as you cannot enlist the connection in the transaction until the previous transaction has + // completed. + DbConnection.EnlistTransaction(current); + } + else + { + // We'll close and reopen the connection to get the benefit of automatic transaction enlistment. + // Remarks: We get here only if there is more than one active query (e.g. nested foreach or two subsequent queries or SaveChanges + // inside a for each) and each of these queries are using a different transaction (note that using TransactionScopeOption.Required + // will not create a new transaction if an ambient transaction already exists - the ambient transaction will be used and we will + // not end up in this code path). If we get here we are already in a loss-loss situation - we cannot enlist to the second transaction + // as this would cause an exception saying that there is already an active transaction that needs to be committed or rolled back + // before we can enlist the connection to a new transaction. The other option (and this is what we do here) is to close and reopen + // the connection. This will enlist the newly opened connection to the second transaction but will also close the reader being used + // by the first active query. As a result when trying to continue reading results from the first query the user will get an exception + // saying that calling "Read" on a closed data reader is not a valid operation. + AmbientTransaction = current; + DbConnection.Close(); + return open(); + } + } + + AmbientTransaction = current; + return Task.CompletedTask; } /// @@ -473,6 +632,7 @@ public virtual bool Close() public virtual SemaphoreSlim Semaphore { get; } = new SemaphoreSlim(1); private readonly List _activeQueries = new List(); + private Transaction _enlistedTransaction; /// /// Registers a potentially bufferable active query. diff --git a/src/EFCore.Relational/Update/Internal/BatchExecutor.cs b/src/EFCore.Relational/Update/Internal/BatchExecutor.cs index 6e30e462991..40d4a4d23c7 100644 --- a/src/EFCore.Relational/Update/Internal/BatchExecutor.cs +++ b/src/EFCore.Relational/Update/Internal/BatchExecutor.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; +using System.Transactions; using JetBrains.Annotations; using Microsoft.EntityFrameworkCore.Internal; using Microsoft.EntityFrameworkCore.Storage; @@ -59,6 +60,8 @@ private int Execute(Tuple, IRelationalConn try { if (connection.CurrentTransaction == null + && connection.EnlistedTransaction == null + && Transaction.Current == null && CurrentContext.Context.Database.AutoTransactionsEnabled) { startedTransaction = connection.BeginTransaction(); @@ -114,6 +117,8 @@ private int Execute(Tuple, IRelationalConn try { if (connection.CurrentTransaction == null + && connection.EnlistedTransaction == null + && Transaction.Current == null && CurrentContext.Context.Database.AutoTransactionsEnabled) { startedTransaction = await connection.BeginTransactionAsync(cancellationToken); diff --git a/src/EFCore.SqlServer/Storage/Internal/SqlServerConnection.cs b/src/EFCore.SqlServer/Storage/Internal/SqlServerConnection.cs index 168f452b498..1213d77b03d 100644 --- a/src/EFCore.SqlServer/Storage/Internal/SqlServerConnection.cs +++ b/src/EFCore.SqlServer/Storage/Internal/SqlServerConnection.cs @@ -60,5 +60,10 @@ public override bool IsMultipleActiveResultSetsEnabled => (bool)(_multipleActiveResultSetsEnabled ?? (_multipleActiveResultSetsEnabled = new SqlConnectionStringBuilder(ConnectionString).MultipleActiveResultSets)); + + /// + /// Indicates whether the store connection supports ambient transactions + /// + protected override bool SupportsAmbientTransactions => true; } } diff --git a/src/EFCore/Infrastructure/DatabaseFacade.cs b/src/EFCore/Infrastructure/DatabaseFacade.cs index d02b9b9d3c1..cc151cca6c9 100644 --- a/src/EFCore/Infrastructure/DatabaseFacade.cs +++ b/src/EFCore/Infrastructure/DatabaseFacade.cs @@ -6,6 +6,7 @@ using System.Linq; using System.Threading; using System.Threading.Tasks; +using System.Transactions; using JetBrains.Annotations; using Microsoft.EntityFrameworkCore.Storage; using Microsoft.EntityFrameworkCore.Utilities; @@ -137,6 +138,13 @@ public virtual void CommitTransaction() public virtual void RollbackTransaction() => TransactionManager.RollbackTransaction(); + /// + /// Specifies an existing to be used for database operations. + /// + /// The transaction to be used. + public virtual void EnlistTransaction([CanBeNull] Transaction transaction) + => TransactionManager.EnlistTransaction(transaction); + /// /// Creates an instance of the configured . /// @@ -144,6 +152,12 @@ public virtual void RollbackTransaction() public virtual IExecutionStrategy CreateExecutionStrategy() => ExecutionStrategyFactory.Create(); + /// + /// The currently enlisted transaction. + /// + public virtual Transaction EnlistedTransaction + => TransactionManager.EnlistedTransaction; + /// /// /// Gets the current being used by the context, or null diff --git a/src/EFCore/Storage/ExecutionStrategy.cs b/src/EFCore/Storage/ExecutionStrategy.cs index 449b72d7b4d..338b475c6cb 100644 --- a/src/EFCore/Storage/ExecutionStrategy.cs +++ b/src/EFCore/Storage/ExecutionStrategy.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; +using System.Transactions; using JetBrains.Annotations; using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.EntityFrameworkCore.Internal; @@ -303,7 +304,9 @@ protected static bool Suspended /// protected virtual void OnFirstExecution() { - if (Dependencies.CurrentDbContext.Context.Database.CurrentTransaction != null) + if (Dependencies.CurrentDbContext.Context.Database.CurrentTransaction != null + || Dependencies.CurrentDbContext.Context.Database.EnlistedTransaction != null + || Transaction.Current != null) { throw new InvalidOperationException( CoreStrings.ExecutionStrategyExistingTransaction( diff --git a/src/EFCore/Storage/IDbContextTransactionManager.cs b/src/EFCore/Storage/IDbContextTransactionManager.cs index 2012f5f8eb7..5f0da5873f5 100644 --- a/src/EFCore/Storage/IDbContextTransactionManager.cs +++ b/src/EFCore/Storage/IDbContextTransactionManager.cs @@ -3,6 +3,8 @@ using System.Threading; using System.Threading.Tasks; +using System.Transactions; +using JetBrains.Annotations; using Microsoft.EntityFrameworkCore.Infrastructure; namespace Microsoft.EntityFrameworkCore.Storage @@ -47,5 +49,17 @@ public interface IDbContextTransactionManager : IResettableService /// Gets the current transaction. /// IDbContextTransaction CurrentTransaction { get; } + + /// + /// The currently enlisted transaction. + /// + Transaction EnlistedTransaction { get; } + + /// + /// Specifies an existing to be used for database operations. + /// + /// The transaction to be used. + void EnlistTransaction([CanBeNull] Transaction transaction); + } } diff --git a/src/EFCore/breakingchanges.netcore.json b/src/EFCore/breakingchanges.netcore.json index a5aba8f2670..de412001478 100644 --- a/src/EFCore/breakingchanges.netcore.json +++ b/src/EFCore/breakingchanges.netcore.json @@ -1,4 +1,4 @@ - [ + [ { "TypeId": "public static class Microsoft.EntityFrameworkCore.Diagnostics.CoreEventId", "MemberId": "public const System.Int32 CoreBaseId = 100000", @@ -18,5 +18,15 @@ "TypeId": "public static class Microsoft.EntityFrameworkCore.Diagnostics.CoreEventId", "MemberId": "public const System.Int32 RelationalBaseId = 200000", "Kind": "Removal" + }, + { + "TypeId": "public interface Microsoft.EntityFrameworkCore.Storage.IDbContextTransactionManager : Microsoft.EntityFrameworkCore.Infrastructure.IResettableService", + "MemberId": "System.Transactions.Transaction get_EnlistedTransaction()", + "Kind": "Addition" + }, + { + "TypeId": "public interface Microsoft.EntityFrameworkCore.Storage.IDbContextTransactionManager : Microsoft.EntityFrameworkCore.Infrastructure.IResettableService", + "MemberId": "System.Void EnlistTransaction(System.Transactions.Transaction transaction)", + "Kind": "Addition" } ] diff --git a/test/EFCore.Relational.Tests/EFCore.Relational.Tests.csproj b/test/EFCore.Relational.Tests/EFCore.Relational.Tests.csproj index 151b7f09e44..31e2e624270 100644 --- a/test/EFCore.Relational.Tests/EFCore.Relational.Tests.csproj +++ b/test/EFCore.Relational.Tests/EFCore.Relational.Tests.csproj @@ -25,6 +25,10 @@ + + + + Component diff --git a/test/EFCore.Relational.Tests/RelationalDatabaseFacadeExtensionsTest.cs b/test/EFCore.Relational.Tests/RelationalDatabaseFacadeExtensionsTest.cs index ea2d226b3f4..d6150f2f2c4 100644 --- a/test/EFCore.Relational.Tests/RelationalDatabaseFacadeExtensionsTest.cs +++ b/test/EFCore.Relational.Tests/RelationalDatabaseFacadeExtensionsTest.cs @@ -163,6 +163,11 @@ public void RollbackTransaction() } public IDbContextTransaction CurrentTransaction { get; } + + public System.Transactions.Transaction EnlistedTransaction { get; } + public void EnlistTransaction(System.Transactions.Transaction transaction) + { + } } [Fact] diff --git a/test/EFCore.Relational.Tests/RelationalEventIdTest.cs b/test/EFCore.Relational.Tests/RelationalEventIdTest.cs index 74f1e00bfc7..cd2feeb644a 100644 --- a/test/EFCore.Relational.Tests/RelationalEventIdTest.cs +++ b/test/EFCore.Relational.Tests/RelationalEventIdTest.cs @@ -24,6 +24,7 @@ using Remotion.Linq.Clauses; using Xunit; +// ReSharper disable InconsistentNaming namespace Microsoft.EntityFrameworkCore { public class RelationalEventIdTest @@ -89,6 +90,9 @@ private class FakeRelationalConnection : IRelationalConnection public int? CommandTimeout { get; set; } public bool IsMultipleActiveResultSetsEnabled => throw new NotImplementedException(); public IDbContextTransaction CurrentTransaction => throw new NotImplementedException(); + public System.Transactions.Transaction EnlistedTransaction { get; } + public void EnlistTransaction(System.Transactions.Transaction transaction) => throw new NotImplementedException(); + public SemaphoreSlim Semaphore => throw new NotImplementedException(); public void RegisterBufferable(IBufferable bufferable) => throw new NotImplementedException(); public Task RegisterBufferableAsync(IBufferable bufferable, CancellationToken cancellationToken) => throw new NotImplementedException(); diff --git a/test/EFCore.SqlServer.FunctionalTests/EFCore.SqlServer.FunctionalTests.csproj b/test/EFCore.SqlServer.FunctionalTests/EFCore.SqlServer.FunctionalTests.csproj index 9bd3ecf834e..7ee0d35d6fe 100644 --- a/test/EFCore.SqlServer.FunctionalTests/EFCore.SqlServer.FunctionalTests.csproj +++ b/test/EFCore.SqlServer.FunctionalTests/EFCore.SqlServer.FunctionalTests.csproj @@ -28,6 +28,10 @@ + + + + diff --git a/test/EFCore.SqlServer.FunctionalTests/SqlServerDatabaseCreatorTest.cs b/test/EFCore.SqlServer.FunctionalTests/SqlServerDatabaseCreatorTest.cs index dd13e810843..0969a8b1ee2 100644 --- a/test/EFCore.SqlServer.FunctionalTests/SqlServerDatabaseCreatorTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/SqlServerDatabaseCreatorTest.cs @@ -7,6 +7,7 @@ using System.Linq; using System.Threading; using System.Threading.Tasks; +using System.Transactions; using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.EntityFrameworkCore.Internal; using Microsoft.EntityFrameworkCore.Storage; @@ -19,36 +20,28 @@ // ReSharper disable InconsistentNaming namespace Microsoft.EntityFrameworkCore { - // Tests are split into classes to enable parralel execution + // Tests are split into classes to enable parallel execution + // Some combinations are skipped to reduce run time public class SqlServerDatabaseCreatorExistsTest { - [ConditionalFact] - public Task Returns_false_when_database_does_not_exist() - { - return Returns_false_when_database_does_not_exist_test(async: false, file: false); - } - - [ConditionalFact] - [SqlServerCondition(SqlServerCondition.SupportsAttach)] - public Task Returns_false_when_database_with_filename_does_not_exist() - { - return Returns_false_when_database_does_not_exist_test(async: false, file: true); - } - - [ConditionalFact] - public Task Async_returns_false_when_database_does_not_exist() + [ConditionalTheory] + [InlineData(true, true)] + [InlineData(false, false)] + public Task Returns_false_when_database_does_not_exist(bool async, bool ambientTransaction) { - return Returns_false_when_database_does_not_exist_test(async: true, file: false); + return Returns_false_when_database_does_not_exist_test(async, ambientTransaction, file: false); } - [ConditionalFact] + [ConditionalTheory] + [InlineData(true, false)] + [InlineData(false, true)] [SqlServerCondition(SqlServerCondition.SupportsAttach)] - public Task Async_returns_false_when_database_with_filename_does_not_exist() + public Task Returns_false_when_database_with_filename_does_not_exist(bool async, bool ambientTransaction) { - return Returns_false_when_database_does_not_exist_test(async: true, file: true); + return Returns_false_when_database_does_not_exist_test(async, ambientTransaction, file: true); } - private static async Task Returns_false_when_database_does_not_exist_test(bool async, bool file) + private static async Task Returns_false_when_database_does_not_exist_test(bool async, bool ambientTransaction, bool file) { using (var testDatabase = SqlServerTestStore.Create("NonExisting", file)) { @@ -56,40 +49,34 @@ private static async Task Returns_false_when_database_does_not_exist_test(bool a { var creator = SqlServerDatabaseCreatorTest.GetDatabaseCreator(context); - Assert.False(async ? await creator.ExistsAsync() : creator.Exists()); + using (SqlServerDatabaseCreatorTest.CreateTransaction(ambientTransaction)) + { + Assert.False(async ? await creator.ExistsAsync() : creator.Exists()); + } Assert.Equal(ConnectionState.Closed, context.Database.GetDbConnection().State); } } } - [ConditionalFact] - public Task Returns_true_when_database_exists() - { - return Returns_true_when_database_exists_test(async: false, file: false); - } - - [ConditionalFact] - [SqlServerCondition(SqlServerCondition.SupportsAttach)] - public Task Returns_true_when_database_with_filename_exists() - { - return Returns_true_when_database_exists_test(async: false, file: true); - } - - [ConditionalFact] - public Task Async_returns_true_when_database_exists() + [ConditionalTheory] + [InlineData(true, false)] + [InlineData(false, true)] + public Task Returns_true_when_database_exists(bool async, bool ambientTransaction) { - return Returns_true_when_database_exists_test(async: true, file: false); + return Returns_true_when_database_exists_test(async, ambientTransaction, file: false); } - [ConditionalFact] + [ConditionalTheory] + [InlineData(true, true)] + [InlineData(false, false)] [SqlServerCondition(SqlServerCondition.SupportsAttach)] - public Task Async_returns_true_when_database_with_filename_exists() + public Task Returns_true_when_database_with_filename_exists(bool async, bool ambientTransaction) { - return Returns_true_when_database_exists_test(async: true, file: true); + return Returns_true_when_database_exists_test(async, ambientTransaction, file: true); } - private static async Task Returns_true_when_database_exists_test(bool async, bool file) + private static async Task Returns_true_when_database_exists_test(bool async, bool ambientTransaction, bool file) { using (var testDatabase = file ? SqlServerTestStore.CreateInitialized("ExistingBloggingFile", useFileName: true) @@ -98,8 +85,10 @@ private static async Task Returns_true_when_database_exists_test(bool async, boo using (var context = new SqlServerDatabaseCreatorTest.BloggingContext(testDatabase)) { var creator = SqlServerDatabaseCreatorTest.GetDatabaseCreator(context); - - Assert.True(async ? await creator.ExistsAsync() : creator.Exists()); + using (SqlServerDatabaseCreatorTest.CreateTransaction(ambientTransaction)) + { + Assert.True(async ? await creator.ExistsAsync() : creator.Exists()); + } Assert.Equal(ConnectionState.Closed, context.Database.GetDbConnection().State); } @@ -109,59 +98,28 @@ private static async Task Returns_true_when_database_exists_test(bool async, boo public class SqlServerDatabaseCreatorEnsureDeletedTest { - [ConditionalFact] - public Task Deletes_database() - { - return Delete_database_test(async: false, open: false, file: false); - } - - [ConditionalFact] - [SqlServerCondition(SqlServerCondition.SupportsAttach)] - public Task Deletes_database_with_filename() - { - return Delete_database_test(async: false, open: false, file: true); - } - - [ConditionalFact] - public Task Async_deletes_database() - { - return Delete_database_test(async: true, open: false, file: false); - } - - [ConditionalFact] - [SqlServerCondition(SqlServerCondition.SupportsAttach)] - public Task Async_deletes_database_with_filename() - { - return Delete_database_test(async: true, open: false, file: true); - } - - [ConditionalFact] - public Task Deletes_database_with_opened_connections() - { - return Delete_database_test(async: false, open: true, file: false); - } - - [ConditionalFact] - [SqlServerCondition(SqlServerCondition.SupportsAttach)] - public Task Deletes_database_with_filename_with_opened_connections() - { - return Delete_database_test(async: false, open: true, file: true); - } - - [ConditionalFact] - public Task Async_deletes_database_with_opened_connections() + [ConditionalTheory] + [InlineData(true, true, true)] + [InlineData(true, false, false)] + [InlineData(false, true, false)] + [InlineData(false, false, true)] + public Task Deletes_database(bool async, bool open, bool ambientTransaction) { - return Delete_database_test(async: true, open: true, file: false); + return Delete_database_test(async, open, ambientTransaction, file: false); } - [ConditionalFact] + [ConditionalTheory] + [InlineData(true, true, false)] + [InlineData(true, false, true)] + [InlineData(false, true, true)] + [InlineData(false, false, false)] [SqlServerCondition(SqlServerCondition.SupportsAttach)] - public Task Async_deletes_database_with_filename_with_opened_connections() + public Task Deletes_database_with_filename(bool async, bool open, bool ambientTransaction) { - return Delete_database_test(async: true, open: true, file: true); + return Delete_database_test(async, open, ambientTransaction, file: true); } - private static async Task Delete_database_test(bool async, bool open, bool file) + private static async Task Delete_database_test(bool async, bool open, bool ambientTransaction, bool file) { using (var testDatabase = SqlServerTestStore.CreateInitialized("EnsureDeleteBlogging" + (file ? "File" : ""), file)) { @@ -176,13 +134,16 @@ private static async Task Delete_database_test(bool async, bool open, bool file) Assert.True(async ? await creator.ExistsAsync() : creator.Exists()); - if (async) + using (SqlServerDatabaseCreatorTest.CreateTransaction(ambientTransaction)) { - Assert.True(await context.Database.EnsureDeletedAsync()); - } - else - { - Assert.True(context.Database.EnsureDeleted()); + if (async) + { + Assert.True(await context.Database.EnsureDeletedAsync()); + } + else + { + Assert.True(context.Database.EnsureDeleted()); + } } Assert.Equal(ConnectionState.Closed, context.Database.GetDbConnection().State); @@ -194,30 +155,21 @@ private static async Task Delete_database_test(bool async, bool open, bool file) } } - [ConditionalFact] - public Task Noop_when_database_does_not_exist() - { - return Noop_when_database_does_not_exist_test(async: false, file: false); - } - - [ConditionalFact] - [SqlServerCondition(SqlServerCondition.SupportsAttach)] - public Task Noop_when_database_with_filename_does_not_exist() - { - return Noop_when_database_does_not_exist_test(async: false, file: true); - } - - [ConditionalFact] - public Task Async_is_noop_when_database_does_not_exist() + [ConditionalTheory] + [InlineData(true)] + [InlineData(false)] + public Task Noop_when_database_does_not_exist(bool async) { - return Noop_when_database_does_not_exist_test(async: true, file: false); + return Noop_when_database_does_not_exist_test(async, file: false); } - [ConditionalFact] + [ConditionalTheory] + [InlineData(true)] + [InlineData(false)] [SqlServerCondition(SqlServerCondition.SupportsAttach)] - public Task Async_is_noop_when_database_with_filename_does_not_exist() + public Task Noop_when_database_with_filename_does_not_exist(bool async) { - return Noop_when_database_does_not_exist_test(async: true, file: true); + return Noop_when_database_does_not_exist_test(async, file: true); } private static async Task Noop_when_database_does_not_exist_test(bool async, bool file) @@ -251,76 +203,57 @@ private static async Task Noop_when_database_does_not_exist_test(bool async, boo public class SqlServerDatabaseCreatorEnsureCreatedTest { - [ConditionalFact] - public Task Creates_schema_in_existing_database() - { - return Creates_schema_in_existing_database_test(async: false, file: false); - } - - [ConditionalFact] - [SqlServerCondition(SqlServerCondition.SupportsAttach)] - public Task Creates_schema_in_existing_database_with_filename() - { - return Creates_schema_in_existing_database_test(async: false, file: true); - } - - [ConditionalFact] - public Task Async_creates_schema_in_existing_database() + [ConditionalTheory] + [InlineData(true, true)] + [InlineData(false, false)] + public Task Creates_schema_in_existing_database(bool async, bool ambientTransaction) { - return Creates_schema_in_existing_database_test(async: true, file: false); + return Creates_schema_in_existing_database_test(async, ambientTransaction, file: false); } - [ConditionalFact] + [ConditionalTheory] + [InlineData(true, false)] + [InlineData(false, true)] [SqlServerCondition(SqlServerCondition.SupportsAttach)] - public Task Async_creates_schema_in_existing_database_with_filename() + public Task Creates_schema_in_existing_database_with_filename(bool async, bool ambientTransaction) { - return Creates_schema_in_existing_database_test(async: true, file: true); + return Creates_schema_in_existing_database_test(async, ambientTransaction, file: true); } - private static Task Creates_schema_in_existing_database_test(bool async, bool file) + private static Task Creates_schema_in_existing_database_test(bool async, bool ambientTransaction, bool file) => TestEnvironment.IsSqlAzure ? new TestSqlServerRetryingExecutionStrategy().ExecuteAsync( - (true, async, file), Creates_physical_database_and_schema_test) - : Creates_physical_database_and_schema_test((true, async, file)); - - [ConditionalFact] - [SqlServerCondition(SqlServerCondition.IsNotSqlAzure)] - public Task Creates_physical_database_and_schema() - { - return Creates_new_physical_database_and_schema_test(async: false, file: false); - } + (true, async, ambientTransaction, file), Creates_physical_database_and_schema_test) + : Creates_physical_database_and_schema_test((true, async, ambientTransaction, file)); - [ConditionalFact] - [SqlServerCondition(SqlServerCondition.SupportsAttach)] - public Task Creates_physical_database_with_filename_and_schema() - { - return Creates_new_physical_database_and_schema_test(async: false, file: true); - } - - [ConditionalFact] + [ConditionalTheory] + [InlineData(true, false)] + [InlineData(false, true)] [SqlServerCondition(SqlServerCondition.IsNotSqlAzure)] - public Task Async_creates_physical_database_and_schema() + public Task Creates_physical_database_and_schema(bool async, bool ambientTransaction) { - return Creates_new_physical_database_and_schema_test(async: true, file: false); + return Creates_new_physical_database_and_schema_test(async, ambientTransaction, file: false); } - [ConditionalFact] + [ConditionalTheory] + [InlineData(true, true)] + [InlineData(false, false)] [SqlServerCondition(SqlServerCondition.SupportsAttach)] - public Task Async_creates_physical_database_with_filename_and_schema() + public Task Creates_physical_database_with_filename_and_schema(bool async, bool ambientTransaction) { - return Creates_new_physical_database_and_schema_test(async: true, file: true); + return Creates_new_physical_database_and_schema_test(async, ambientTransaction, file: true); } - private static Task Creates_new_physical_database_and_schema_test(bool async, bool file) + private static Task Creates_new_physical_database_and_schema_test(bool async, bool ambientTransaction, bool file) => TestEnvironment.IsSqlAzure ? new TestSqlServerRetryingExecutionStrategy().ExecuteAsync( - (false, async, file), Creates_physical_database_and_schema_test) - : Creates_physical_database_and_schema_test((false, async, file)); + (false, async, ambientTransaction, file), Creates_physical_database_and_schema_test) + : Creates_physical_database_and_schema_test((false, async, ambientTransaction, file)); private static async Task Creates_physical_database_and_schema_test( - (bool CreateDatabase, bool Async, bool File) options) + (bool CreateDatabase, bool Async, bool ambientTransaction, bool File) options) { - (bool createDatabase, bool async, bool file) = options; + (bool createDatabase, bool async, bool ambientTransaction, bool file) = options; using (var testDatabase = SqlServerTestStore.Create("EnsureCreatedTest" + (file ? "File" : ""), file)) { using (var context = new SqlServerDatabaseCreatorTest.BloggingContext(testDatabase)) @@ -338,13 +271,16 @@ private static Task Creates_new_physical_database_and_schema_test(bool async, bo Assert.Equal(ConnectionState.Closed, context.Database.GetDbConnection().State); - if (async) - { - Assert.True(await creator.EnsureCreatedAsync()); - } - else + using (SqlServerDatabaseCreatorTest.CreateTransaction(ambientTransaction)) { - Assert.True(creator.EnsureCreated()); + if (async) + { + Assert.True(await creator.EnsureCreatedAsync()); + } + else + { + Assert.True(creator.EnsureCreated()); + } } Assert.Equal(ConnectionState.Closed, context.Database.GetDbConnection().State); @@ -386,30 +322,21 @@ private static Task Creates_new_physical_database_and_schema_test(bool async, bo } } - [ConditionalFact] - public Task Noop_when_database_exists_and_has_schema() - { - return Noop_when_database_exists_and_has_schema_test(async: false, file: false); - } - - [ConditionalFact] - [SqlServerCondition(SqlServerCondition.SupportsAttach)] - public Task Noop_when_database_with_filename_exists_and_has_schema() - { - return Noop_when_database_exists_and_has_schema_test(async: false, file: true); - } - - [ConditionalFact] - public Task Async_is_noop_when_database_exists_and_has_schema() + [ConditionalTheory] + [InlineData(true)] + [InlineData(false)] + public Task Noop_when_database_exists_and_has_schema(bool async) { - return Noop_when_database_exists_and_has_schema_test(async: true, file: false); + return Noop_when_database_exists_and_has_schema_test(async, file: false); } - [ConditionalFact] + [ConditionalTheory] + [InlineData(true)] + [InlineData(false)] [SqlServerCondition(SqlServerCondition.SupportsAttach)] - public Task Async_is_noop_when_database_with_filename_exists_and_has_schema() + public Task Noop_when_database_with_filename_exists_and_has_schema(bool async) { - return Noop_when_database_exists_and_has_schema_test(async: true, file: true); + return Noop_when_database_exists_and_has_schema_test(async, file: true); } private static async Task Noop_when_database_exists_and_has_schema_test(bool async, bool file) @@ -437,19 +364,10 @@ private static async Task Noop_when_database_exists_and_has_schema_test(bool asy public class SqlServerDatabaseCreatorHasTablesTest { - [ConditionalFact] - public Task Throws_when_database_does_not_exist() - { - return Throws_when_database_does_not_exist_test(async: false); - } - - [ConditionalFact] - public Task Async_throws_when_database_does_not_exist() - { - return Throws_when_database_does_not_exist_test(async: true); - } - - private static async Task Throws_when_database_does_not_exist_test(bool async) + [ConditionalTheory] + [InlineData(true)] + [InlineData(false)] + public async Task Throws_when_database_does_not_exist(bool async) { using (var testDatabase = SqlServerTestStore.GetOrCreate("NonExisting")) { @@ -472,65 +390,44 @@ private static async Task Throws_when_database_does_not_exist_test(bool async) } } - [ConditionalFact] - public Task Returns_false_when_database_exists_but_has_no_tables() - { - return Returns_false_when_database_exists_but_has_no_tables_test(async: false); - } - - [ConditionalFact] - public Task Async_returns_false_when_database_exists_but_has_no_tables() - { - return Returns_false_when_database_exists_but_has_no_tables_test(async: true); - } - - private static async Task Returns_false_when_database_exists_but_has_no_tables_test(bool async) + [ConditionalTheory] + [InlineData(true, false)] + [InlineData(false, true)] + public async Task Returns_false_when_database_exists_but_has_no_tables(bool async, bool ambientTransaction) { using (var testDatabase = SqlServerTestStore.GetOrCreateInitialized("Empty")) { var creator = SqlServerDatabaseCreatorTest.GetDatabaseCreator(testDatabase); - Assert.False(async ? await creator.HasTablesAsyncBase() : creator.HasTablesBase()); + using (SqlServerDatabaseCreatorTest.CreateTransaction(ambientTransaction)) + { + Assert.False(async ? await creator.HasTablesAsyncBase() : creator.HasTablesBase()); + } } } - [ConditionalFact] - public Task Returns_true_when_database_exists_and_has_any_tables() - { - return Returns_true_when_database_exists_and_has_any_tables_test(async: false); - } - - [ConditionalFact] - public Task Async_returns_true_when_database_exists_and_has_any_tables() - { - return Returns_true_when_database_exists_and_has_any_tables_test(async: true); - } - - private static async Task Returns_true_when_database_exists_and_has_any_tables_test(bool async) + [ConditionalTheory] + [InlineData(true, true)] + [InlineData(false, false)] + public async Task Returns_true_when_database_exists_and_has_any_tables(bool async, bool ambientTransaction) { using (var testDatabase = SqlServerTestStore.GetOrCreate("ExistingTables") .InitializeSqlServer(null, t => new SqlServerDatabaseCreatorTest.BloggingContext(t), null)) { var creator = SqlServerDatabaseCreatorTest.GetDatabaseCreator(testDatabase); - Assert.True(async ? await creator.HasTablesAsyncBase() : creator.HasTablesBase()); + using (SqlServerDatabaseCreatorTest.CreateTransaction(ambientTransaction)) + { + Assert.True(async ? await creator.HasTablesAsyncBase() : creator.HasTablesBase()); + } } } } public class SqlServerDatabaseCreatorDeleteTest { - [ConditionalFact] - public async Task Deletes_database() - { - await Deletes_database_test(async: false); - } - - [ConditionalFact] - public async Task Async_deletes_database() - { - await Deletes_database_test(async: true); - } - - private static async Task Deletes_database_test(bool async) + [ConditionalTheory] + [InlineData(true, true)] + [InlineData(false, false)] + public static async Task Deletes_database(bool async, bool ambientTransaction) { using (var testDatabase = SqlServerTestStore.CreateInitialized("DeleteBlogging")) { @@ -540,32 +437,26 @@ private static async Task Deletes_database_test(bool async) Assert.True(async ? await creator.ExistsAsync() : creator.Exists()); - if (async) - { - await creator.DeleteAsync(); - } - else + using (SqlServerDatabaseCreatorTest.CreateTransaction(ambientTransaction)) { - creator.Delete(); + if (async) + { + await creator.DeleteAsync(); + } + else + { + creator.Delete(); + } } Assert.False(async ? await creator.ExistsAsync() : creator.Exists()); } } - [ConditionalFact] - public Task Throws_when_database_does_not_exist() - { - return Throws_when_database_does_not_exist_test(async: false); - } - - [ConditionalFact] - public Task Async_throws_when_database_does_not_exist() - { - return Throws_when_database_does_not_exist_test(async: true); - } - - private static async Task Throws_when_database_does_not_exist_test(bool async) + [ConditionalTheory] + [InlineData(true)] + [InlineData(false)] + public async Task Throws_when_database_does_not_exist(bool async) { using (var testDatabase = SqlServerTestStore.GetOrCreate("NonExistingBlogging")) { @@ -598,19 +489,10 @@ public void Throws_when_no_initial_catalog() public class SqlServerDatabaseCreatorCreateTablesTest { - [ConditionalFact] - public Task Creates_schema_in_existing_database() - { - return Creates_schema_in_existing_database_test(async: false); - } - - [ConditionalFact] - public Task Async_creates_schema_in_existing_database() - { - return Creates_schema_in_existing_database_test(async: true); - } - - private static async Task Creates_schema_in_existing_database_test(bool async) + [ConditionalTheory] + [InlineData(true, true)] + [InlineData(false, false)] + public async Task Creates_schema_in_existing_database_test(bool async, bool ambientTransaction) { using (var testDatabase = SqlServerTestStore.GetOrCreateInitialized("ExistingBlogging" + (async ? "Async" : ""))) { @@ -618,13 +500,16 @@ private static async Task Creates_schema_in_existing_database_test(bool async) { var creator = SqlServerDatabaseCreatorTest.GetDatabaseCreator(context); - if (async) + using (SqlServerDatabaseCreatorTest.CreateTransaction(ambientTransaction)) { - await creator.CreateTablesAsync(); - } - else - { - creator.CreateTables(); + if (async) + { + await creator.CreateTablesAsync(); + } + else + { + creator.CreateTables(); + } } if (testDatabase.ConnectionState != ConnectionState.Open) @@ -658,19 +543,10 @@ private static async Task Creates_schema_in_existing_database_test(bool async) } } - [ConditionalFact] - public Task Throws_if_database_does_not_exist() - { - return Throws_if_database_does_not_exist_test(async: false); - } - - [ConditionalFact] - public Task Async_throws_if_database_does_not_exist() - { - return Throws_if_database_does_not_exist_test(async: true); - } - - private static async Task Throws_if_database_does_not_exist_test(bool async) + [ConditionalTheory] + [InlineData(true)] + [InlineData(false)] + public async Task Throws_if_database_does_not_exist(bool async) { using (var testDatabase = SqlServerTestStore.GetOrCreate("NonExisting")) { @@ -693,19 +569,10 @@ var errorNumber public class SqlServerDatabaseCreatorCreateTest { - [ConditionalFact] - public Task Creates_physical_database_but_not_tables() - { - return Creates_physical_database_but_not_tables_test(async: false); - } - - [ConditionalFact] - public Task Async_creates_physical_database_but_not_tables() - { - return Creates_physical_database_but_not_tables_test(async: true); - } - - private static async Task Creates_physical_database_but_not_tables_test(bool async) + [ConditionalTheory] + [InlineData(true, false)] + [InlineData(false, true)] + public async Task Creates_physical_database_but_not_tables(bool async, bool ambientTransaction) { using (var testDatabase = SqlServerTestStore.GetOrCreate("CreateTest")) { @@ -713,13 +580,16 @@ private static async Task Creates_physical_database_but_not_tables_test(bool asy creator.EnsureDeleted(); - if (async) - { - await creator.CreateAsync(); - } - else + using (SqlServerDatabaseCreatorTest.CreateTransaction(ambientTransaction)) { - creator.Create(); + if (async) + { + await creator.CreateAsync(); + } + else + { + creator.Create(); + } } Assert.True(creator.Exists()); @@ -742,19 +612,10 @@ private static async Task Creates_physical_database_but_not_tables_test(bool asy } } - [ConditionalFact] - public Task Throws_if_database_already_exists() - { - return Throws_if_database_already_exists_test(async: false); - } - - [ConditionalFact] - public Task Async_throws_if_database_already_exists() - { - return Throws_if_database_already_exists_test(async: true); - } - - private static async Task Throws_if_database_already_exists_test(bool async) + [ConditionalTheory] + [InlineData(true)] + [InlineData(false)] + public async Task Throws_if_database_already_exists(bool async) { using (var testDatabase = SqlServerTestStore.GetOrCreateInitialized("ExistingBlogging")) { @@ -800,6 +661,19 @@ private static IServiceProvider CreateServiceProvider() .AddScoped() .BuildServiceProvider(); + + public static TransactionScope CreateTransaction(bool useTransaction) + { + if (useTransaction) + { +#if NET461 + return new TransactionScope(TransactionScopeAsyncFlowOption.Enabled); +#endif + } + + return new TransactionScope(TransactionScopeOption.Suppress, TransactionScopeAsyncFlowOption.Enabled); + } + public class BloggingContext : DbContext { private readonly string _connectionString; diff --git a/test/EFCore.SqlServer.FunctionalTests/TestUtilities/TestSqlServerConnection.cs b/test/EFCore.SqlServer.FunctionalTests/TestUtilities/TestSqlServerConnection.cs index b82163f7f11..2b8457370f6 100644 --- a/test/EFCore.SqlServer.FunctionalTests/TestUtilities/TestSqlServerConnection.cs +++ b/test/EFCore.SqlServer.FunctionalTests/TestUtilities/TestSqlServerConnection.cs @@ -43,6 +43,11 @@ protected TestSqlServerConnection(ISqlServerConnection connection) } public virtual IDbContextTransaction CurrentTransaction { get; private set; } + public System.Transactions.Transaction EnlistedTransaction { get; } + public void EnlistTransaction(System.Transactions.Transaction transaction) + { + throw new NotImplementedException(); + } public SemaphoreSlim Semaphore => new SemaphoreSlim(1); diff --git a/test/EFCore.SqlServer.FunctionalTests/TransactionSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/TransactionSqlServerTest.cs index 4fe23e6dad8..28ca1635378 100644 --- a/test/EFCore.SqlServer.FunctionalTests/TransactionSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/TransactionSqlServerTest.cs @@ -18,6 +18,10 @@ public TransactionSqlServerTest(TransactionSqlServerFixture fixture) protected override bool SnapshotSupported => true; +#if NET461 + protected override bool AmbientTransactionsSupported => true; +#endif + public virtual void Dispose() { TestSqlServerRetryingExecutionStrategy.Suspended = false; diff --git a/test/EFCore.Sqlite.FunctionalTests/EFCore.Sqlite.FunctionalTests.csproj b/test/EFCore.Sqlite.FunctionalTests/EFCore.Sqlite.FunctionalTests.csproj index ef5ff27d3f1..7e5328d374c 100644 --- a/test/EFCore.Sqlite.FunctionalTests/EFCore.Sqlite.FunctionalTests.csproj +++ b/test/EFCore.Sqlite.FunctionalTests/EFCore.Sqlite.FunctionalTests.csproj @@ -25,6 +25,10 @@ + + + + diff --git a/test/EFCore.Sqlite.FunctionalTests/Query/BadDataSqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/Query/BadDataSqliteTest.cs index e78da7fa152..85c8f331a6d 100644 --- a/test/EFCore.Sqlite.FunctionalTests/Query/BadDataSqliteTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/Query/BadDataSqliteTest.cs @@ -365,6 +365,8 @@ private class FakeConnection : IRelationalConnection public void CommitTransaction() { } public void RollbackTransaction() { } public IDbContextTransaction CurrentTransaction => throw new NotImplementedException(); + public System.Transactions.Transaction EnlistedTransaction { get; } + public void EnlistTransaction(System.Transactions.Transaction transaction) => throw new NotImplementedException(); public SemaphoreSlim Semaphore { get; } public void RegisterBufferable(IBufferable bufferable) { } public Task RegisterBufferableAsync(IBufferable bufferable, CancellationToken cancellationToken) => throw new NotImplementedException(); diff --git a/test/EFCore.Sqlite.Tests/EFCore.Sqlite.Tests.csproj b/test/EFCore.Sqlite.Tests/EFCore.Sqlite.Tests.csproj index 99f23584dbb..fc91db96ffe 100644 --- a/test/EFCore.Sqlite.Tests/EFCore.Sqlite.Tests.csproj +++ b/test/EFCore.Sqlite.Tests/EFCore.Sqlite.Tests.csproj @@ -26,6 +26,10 @@ + + + + diff --git a/test/EFCore.Tests/DatabaseFacadeTest.cs b/test/EFCore.Tests/DatabaseFacadeTest.cs index f839282f4ae..838a09ca870 100644 --- a/test/EFCore.Tests/DatabaseFacadeTest.cs +++ b/test/EFCore.Tests/DatabaseFacadeTest.cs @@ -144,6 +144,8 @@ public Task BeginTransactionAsync(CancellationToken cance public void CommitTransaction() => CommitCalls++; public void RollbackTransaction() => RollbackCalls++; public IDbContextTransaction CurrentTransaction => _transaction; + public System.Transactions.Transaction EnlistedTransaction { get; } + public void EnlistTransaction(System.Transactions.Transaction transaction) => throw new NotImplementedException(); public void ResetState() => throw new NotImplementedException(); } diff --git a/test/EFCore.Tests/EFCore.Tests.csproj b/test/EFCore.Tests/EFCore.Tests.csproj index e3bd05d640e..f0cd195d965 100644 --- a/test/EFCore.Tests/EFCore.Tests.csproj +++ b/test/EFCore.Tests/EFCore.Tests.csproj @@ -20,6 +20,10 @@ + + + + diff --git a/test/EFCore.Tests/Storage/ExecutionStrategyTest.cs b/test/EFCore.Tests/Storage/ExecutionStrategyTest.cs index d16a041e995..791c73d0d7d 100644 --- a/test/EFCore.Tests/Storage/ExecutionStrategyTest.cs +++ b/test/EFCore.Tests/Storage/ExecutionStrategyTest.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; +using System.Transactions; using Microsoft.EntityFrameworkCore.Internal; using Microsoft.EntityFrameworkCore.TestUtilities; using Microsoft.Extensions.DependencyInjection; @@ -14,51 +15,17 @@ // ReSharper disable InconsistentNaming namespace Microsoft.EntityFrameworkCore.Storage { - public class ExecutionStrategyTest + public class ExecutionStrategyTest : IDisposable { - private class TestExecutionStrategy : ExecutionStrategy - { - private readonly Func _shouldRetryOn; - private readonly Func _getNextDelay; - - public TestExecutionStrategy( - DbContext context, - int? retryCount = null, - Func shouldRetryOn = null, - Func getNextDelay = null) - : base( - context, - retryCount ?? DefaultMaxRetryCount, - DefaultMaxDelay) - { - _shouldRetryOn = shouldRetryOn; - _getNextDelay = getNextDelay; - } - - protected internal override bool ShouldRetryOn(Exception exception) - => _shouldRetryOn?.Invoke(exception) == true; + private readonly DbContext Context; - protected override TimeSpan? GetNextDelay(Exception lastException) - { - var baseDelay = base.GetNextDelay(lastException); - return baseDelay != null && _getNextDelay != null ? _getNextDelay.Invoke(lastException) : baseDelay; - } + public ExecutionStrategyTest() => Context = CreateContext(); - public TimeSpan? GetNextDelayBase(Exception lastException) - { - ExceptionsEncountered.Add(lastException); - return base.GetNextDelay(lastException); - } - - public new static bool Suspended - { - set => ExecutionStrategy.Suspended = value; - } - } + public void Dispose() => Context.Dispose(); private TestExecutionStrategy CreateFailOnRetryStrategy() => new TestExecutionStrategy( - CreateContext(), + Context, shouldRetryOn: e => { Assert.True(false); @@ -73,7 +40,7 @@ private TestExecutionStrategy CreateFailOnRetryStrategy() [Fact] public void GetNextDelay_returns_the_expected_default_sequence() { - var strategy = new TestExecutionStrategy(CreateContext()); + var strategy = new TestExecutionStrategy(Context); var delays = new List(); var delay = strategy.GetNextDelayBase(new Exception()); while (delay != null) @@ -105,7 +72,7 @@ public void GetNextDelay_returns_the_expected_default_sequence() [Fact] public void RetriesOnFailure_returns_true() { - var mockExecutionStrategy = new TestExecutionStrategy(CreateContext()); + var mockExecutionStrategy = new TestExecutionStrategy(Context); Assert.True(mockExecutionStrategy.RetriesOnFailure); } @@ -122,17 +89,67 @@ public void Execute_Func_throws_for_an_existing_transaction() Execute_throws_for_an_existing_transaction(e => e.Execute(() => 1)); } - private void Execute_throws_for_an_existing_transaction(Action executeAsync) + private void Execute_throws_for_an_existing_transaction(Action execute) + { + var mockExecutionStrategy = new TestExecutionStrategy(Context); + using (Context.Database.BeginTransaction()) + { + Assert.Equal( + CoreStrings.ExecutionStrategyExistingTransaction(mockExecutionStrategy.GetType().Name, "DbContext.Database.CreateExecutionStrategy()"), + Assert.Throws( + () => execute(mockExecutionStrategy)) + .Message); + } + } + + [Fact] + public void Execute_Action_throws_for_an_ambient_transaction() + { + Execute_throws_for_an_ambient_transaction(e => e.Execute(() => { })); + } + + [Fact] + public void Execute_Func_throws_for_an_ambient_transaction() + { + Execute_throws_for_an_ambient_transaction(e => e.Execute(() => 1)); + } + + private void Execute_throws_for_an_ambient_transaction(Action execute) + { + var mockExecutionStrategy = new TestExecutionStrategy(Context); + using (new TransactionScope()) + { + Assert.Equal( + CoreStrings.ExecutionStrategyExistingTransaction(mockExecutionStrategy.GetType().Name, "DbContext.Database.CreateExecutionStrategy()"), + Assert.Throws( + () => execute(mockExecutionStrategy)) + .Message); + } + } + + [Fact] + public void Execute_Action_throws_for_an_enlisted_transaction() { - var context = CreateContext(); - var mockExecutionStrategy = new TestExecutionStrategy(context); - using (context.Database.BeginTransaction()) + Execute_throws_for_an_enlisted_transaction(e => e.Execute(() => { })); + } + + [Fact] + public void Execute_Func_throws_for_an_enlisted_transaction() + { + Execute_throws_for_an_enlisted_transaction(e => e.Execute(() => 1)); + } + + private void Execute_throws_for_an_enlisted_transaction(Action execute) + { + var mockExecutionStrategy = new TestExecutionStrategy(Context); + using (var t = new CommittableTransaction()) { + Context.Database.EnlistTransaction(t); + Assert.Equal( CoreStrings.ExecutionStrategyExistingTransaction(mockExecutionStrategy.GetType().Name, "DbContext.Database.CreateExecutionStrategy()"), Assert.Throws( - () => - executeAsync(mockExecutionStrategy)) + () => execute(mockExecutionStrategy)) .Message); } } @@ -154,7 +171,7 @@ private void Execute_does_not_throw_when_invoked_twice(Action e is ArgumentOutOfRangeException); for (var i = 0; i < 2; i++) @@ -239,7 +256,7 @@ public void Execute_Func_retries_until_succesful() private void Execute_retries_until_succesful(Action> execute) { var executionStrategyMock = new TestExecutionStrategy( - CreateContext(), + Context, shouldRetryOn: e => e is ArgumentOutOfRangeException, getNextDelay: e => TimeSpan.FromTicks(0)); @@ -274,7 +291,7 @@ public void Execute_Func_retries_until_not_retrieable_exception_is_thrown() private void Execute_retries_until_not_retrieable_exception_is_thrown(Action> execute) { var executionStrategyMock = new TestExecutionStrategy( - CreateContext(), + Context, shouldRetryOn: e => e is ArgumentOutOfRangeException, getNextDelay: e => TimeSpan.FromTicks(0)); @@ -312,24 +329,24 @@ private void Execute_retries_until_limit_is_reached(Action e is ArgumentOutOfRangeException, getNextDelay: e => TimeSpan.FromTicks(0)); Assert.IsType( Assert.Throws( - () => - execute( - executionStrategyMock, () => + () => + execute( + executionStrategyMock, () => + { + if (executionCount++ < 3) { - if (executionCount++ < 3) - { - throw new ArgumentOutOfRangeException(); - } - Assert.True(false); - return 0; - })) + throw new ArgumentOutOfRangeException(); + } + Assert.True(false); + return 0; + })) .InnerException); Assert.Equal(3, executionCount); @@ -349,15 +366,65 @@ public Task ExecuteAsync_Func_throws_for_an_existing_transaction() private async Task ExecuteAsync_throws_for_an_existing_transaction(Func executeAsync) { - var context = CreateContext(); - var mockExecutionStrategy = new TestExecutionStrategy(context); - using (context.Database.BeginTransaction()) + var mockExecutionStrategy = new TestExecutionStrategy(Context); + using (Context.Database.BeginTransaction()) + { + Assert.Equal( + CoreStrings.ExecutionStrategyExistingTransaction(mockExecutionStrategy.GetType().Name, "DbContext.Database.CreateExecutionStrategy()"), + (await Assert.ThrowsAsync( + () => executeAsync(mockExecutionStrategy))).Message); + } + } + + [Fact] + public void ExecuteAsync_Action_throws_for_an_ambient_transaction() + { + ExecuteAsync_throws_for_an_ambient_transaction(e => e.ExecuteAsync(() => (Task)Task.FromResult(1))); + } + + [Fact] + public void ExecuteAsync_Func_throws_for_an_ambient_transaction() + { + ExecuteAsync_throws_for_an_ambient_transaction(e => e.ExecuteAsync(ct => Task.FromResult(1), CancellationToken.None)); + } + + private async void ExecuteAsync_throws_for_an_ambient_transaction(Func executeAsync) + { + var mockExecutionStrategy = new TestExecutionStrategy(Context); + using (new TransactionScope(TransactionScopeAsyncFlowOption.Enabled)) + { + Assert.Equal( + CoreStrings.ExecutionStrategyExistingTransaction(mockExecutionStrategy.GetType().Name, "DbContext.Database.CreateExecutionStrategy()"), + (await Assert.ThrowsAsync( + () => executeAsync(mockExecutionStrategy))) + .Message); + } + } + + [Fact] + public void ExecuteAsync_Action_throws_for_an_enlisted_transaction() + { + ExecuteAsync_throws_for_an_enlisted_transaction(e => e.ExecuteAsync(() => (Task)Task.FromResult(1))); + } + + [Fact] + public void ExecuteAsync_Func_throws_for_an_enlisted_transaction() + { + ExecuteAsync_throws_for_an_enlisted_transaction(e => e.ExecuteAsync(ct => Task.FromResult(1), CancellationToken.None)); + } + + private async void ExecuteAsync_throws_for_an_enlisted_transaction(Func executeAsync) + { + var mockExecutionStrategy = new TestExecutionStrategy(Context); + using (var t = new CommittableTransaction()) { + Context.Database.EnlistTransaction(t); + Assert.Equal( CoreStrings.ExecutionStrategyExistingTransaction(mockExecutionStrategy.GetType().Name, "DbContext.Database.CreateExecutionStrategy()"), (await Assert.ThrowsAsync( - () => - executeAsync(mockExecutionStrategy))).Message); + () => executeAsync(mockExecutionStrategy))) + .Message); } } @@ -378,7 +445,7 @@ private async Task ExecuteAsync_does_not_throw_when_invoked_twice(Func e is ArgumentOutOfRangeException); for (var i = 0; i < 2; i++) @@ -463,7 +530,7 @@ public Task ExecuteAsync_Func_retries_until_succesful() private async Task ExecuteAsync_retries_until_succesful(Func>, Task> executeAsync) { var executionStrategyMock = new TestExecutionStrategy( - CreateContext(), + Context, shouldRetryOn: e => e is ArgumentOutOfRangeException, getNextDelay: e => TimeSpan.FromTicks(0)); @@ -500,7 +567,7 @@ public Task ExecuteAsync_Func_retries_until_not_retrieable_exception_is_thrown() Func>, Task> executeAsync) { var executionStrategyMock = new TestExecutionStrategy( - CreateContext(), + Context, shouldRetryOn: e => e is ArgumentOutOfRangeException, getNextDelay: e => TimeSpan.FromTicks(0)); @@ -537,7 +604,7 @@ private async Task ExecuteAsync_retries_until_limit_is_reached(Func e is ArgumentOutOfRangeException, getNextDelay: e => TimeSpan.FromTicks(0)); @@ -566,5 +633,45 @@ protected DbContext CreateContext() new ServiceCollection() .AddScoped()), InMemoryTestHelpers.Instance.CreateOptions()); + + private class TestExecutionStrategy : ExecutionStrategy + { + private readonly Func _shouldRetryOn; + private readonly Func _getNextDelay; + + public TestExecutionStrategy( + DbContext context, + int? retryCount = null, + Func shouldRetryOn = null, + Func getNextDelay = null) + : base( + context, + retryCount ?? DefaultMaxRetryCount, + DefaultMaxDelay) + { + _shouldRetryOn = shouldRetryOn; + _getNextDelay = getNextDelay; + } + + protected internal override bool ShouldRetryOn(Exception exception) + => _shouldRetryOn?.Invoke(exception) == true; + + protected override TimeSpan? GetNextDelay(Exception lastException) + { + var baseDelay = base.GetNextDelay(lastException); + return baseDelay != null && _getNextDelay != null ? _getNextDelay.Invoke(lastException) : baseDelay; + } + + public TimeSpan? GetNextDelayBase(Exception lastException) + { + ExceptionsEncountered.Add(lastException); + return base.GetNextDelay(lastException); + } + + public new static bool Suspended + { + set => ExecutionStrategy.Suspended = value; + } + } } } diff --git a/test/EFCore.Tests/TestUtilities/TestInMemoryTransactionManager.cs b/test/EFCore.Tests/TestUtilities/TestInMemoryTransactionManager.cs index 4fd33882a8a..1d311d31c31 100644 --- a/test/EFCore.Tests/TestUtilities/TestInMemoryTransactionManager.cs +++ b/test/EFCore.Tests/TestUtilities/TestInMemoryTransactionManager.cs @@ -4,6 +4,7 @@ using System; using System.Threading; using System.Threading.Tasks; +using System.Transactions; using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.EntityFrameworkCore.Storage; using Microsoft.EntityFrameworkCore.Storage.Internal; @@ -13,6 +14,7 @@ namespace Microsoft.EntityFrameworkCore.TestUtilities public class TestInMemoryTransactionManager : InMemoryTransactionManager { private IDbContextTransaction _currentTransaction; + private Transaction _enlistedTransaction; public TestInMemoryTransactionManager( IDiagnosticsLogger logger) @@ -22,22 +24,19 @@ public class TestInMemoryTransactionManager : InMemoryTransactionManager public override IDbContextTransaction CurrentTransaction => _currentTransaction; - public override IDbContextTransaction BeginTransaction() - { - _currentTransaction = new TestInMemoryTransaction(this); - return _currentTransaction; - } + public override Transaction EnlistedTransaction => _enlistedTransaction; + + public override IDbContextTransaction BeginTransaction() => _currentTransaction = new TestInMemoryTransaction(this); public override Task BeginTransactionAsync(CancellationToken cancellationToken = default(CancellationToken)) - { - _currentTransaction = new TestInMemoryTransaction(this); - return Task.FromResult(_currentTransaction); - } + => Task.FromResult(_currentTransaction = new TestInMemoryTransaction(this)); public override void CommitTransaction() => CurrentTransaction.Commit(); public override void RollbackTransaction() => CurrentTransaction.Rollback(); + public override void EnlistTransaction(Transaction transaction) => _enlistedTransaction = transaction; + private class TestInMemoryTransaction : IDbContextTransaction { public TestInMemoryTransaction(TestInMemoryTransactionManager transactionManager) @@ -45,7 +44,7 @@ public TestInMemoryTransaction(TestInMemoryTransactionManager transactionManager TransactionManager = transactionManager; } - public virtual Guid TransactionId { get; } = Guid.NewGuid(); + public Guid TransactionId { get; } = Guid.NewGuid(); private TestInMemoryTransactionManager TransactionManager { get; }