Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow opting out of automatic savepoints in SaveChanges #23698

Merged
merged 2 commits into from
Dec 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/EFCore.Relational/Update/Internal/BatchExecutor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@ public virtual int Execute(
{
connection.Open();

if (transaction?.SupportsSavepoints == true)
if (transaction?.SupportsSavepoints == true
&& CurrentContext.Context.Database.AutoSavepointsEnabled)
{
transaction.CreateSavepoint(SavepointName);
createdSavepoint = true;
Expand Down Expand Up @@ -181,7 +182,8 @@ public virtual async Task<int> ExecuteAsync(
{
await connection.OpenAsync(cancellationToken).ConfigureAwait(false);

if (transaction?.SupportsSavepoints == true)
if (transaction?.SupportsSavepoints == true
&& CurrentContext.Context.Database.AutoSavepointsEnabled)
{
await transaction.CreateSavepointAsync(SavepointName, cancellationToken).ConfigureAwait(false);
createdSavepoint = true;
Expand Down
5 changes: 5 additions & 0 deletions src/EFCore/DbContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -718,6 +718,10 @@ void IDbContextPoolable.SetLease(DbContextLease lease)
_database.AutoTransactionsEnabled
= _configurationSnapshot?.AutoTransactionsEnabled == null
|| _configurationSnapshot.AutoTransactionsEnabled.Value;

_database.AutoSavepointsEnabled
= _configurationSnapshot?.AutoSavepointsEnabled == null
|| _configurationSnapshot.AutoSavepointsEnabled.Value;
}
}

Expand All @@ -733,6 +737,7 @@ void IDbContextPoolable.SnapshotConfiguration()
_changeTracker?.AutoDetectChangesEnabled,
_changeTracker?.QueryTrackingBehavior,
_database?.AutoTransactionsEnabled,
_database?.AutoSavepointsEnabled,
_changeTracker?.LazyLoadingEnabled,
_changeTracker?.CascadeDeleteTiming,
_changeTracker?.DeleteOrphansTiming);
Expand Down
17 changes: 17 additions & 0 deletions src/EFCore/Infrastructure/DatabaseFacade.cs
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,23 @@ public virtual IDbContextTransaction CurrentTransaction
/// </summary>
public virtual bool AutoTransactionsEnabled { get; set; } = true;

/// <summary>
/// <para>
/// Whether a transaction savepoint will be created automatically by <see cref="DbContext.SaveChanges()" /> if it is called
/// after a transaction has been manually started with <see cref="BeginTransaction" />.
/// </para>
/// <para>
/// The default value is <see langword="true" />, meaning that <see cref="DbContext.SaveChanges()" /> will create a
/// transaction savepoint within a manually-started transaction. Regardless of this property, savepoints are only created
/// if the data provider supports them; see <see cref="IDbContextTransaction.SupportsSavepoints"/>.
/// </para>
/// <para>
/// Setting this value to <see langword="false" /> should only be done with caution since the database could be left in a
/// corrupted state if <see cref="DbContext.SaveChanges()" /> fails.
/// </para>
/// </summary>
public virtual bool AutoSavepointsEnabled { get; set; } = true;
roji marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// <para>
/// Returns the name of the database provider currently in use.
Expand Down
10 changes: 10 additions & 0 deletions src/EFCore/Internal/DbContextPoolConfigurationSnapshot.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,15 @@ public DbContextPoolConfigurationSnapshot(
bool? autoDetectChangesEnabled,
QueryTrackingBehavior? queryTrackingBehavior,
bool? autoTransactionsEnabled,
bool? autoSavepointsEnabled,
bool? lazyLoadingEnabled,
CascadeTiming? cascadeDeleteTiming,
CascadeTiming? deleteOrphansTiming)
{
AutoDetectChangesEnabled = autoDetectChangesEnabled;
QueryTrackingBehavior = queryTrackingBehavior;
AutoTransactionsEnabled = autoTransactionsEnabled;
AutoSavepointsEnabled = autoSavepointsEnabled;
LazyLoadingEnabled = lazyLoadingEnabled;
CascadeDeleteTiming = cascadeDeleteTiming;
DeleteOrphansTiming = deleteOrphansTiming;
Expand Down Expand Up @@ -82,5 +84,13 @@ public DbContextPoolConfigurationSnapshot(
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public virtual bool? AutoTransactionsEnabled { get; }

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public virtual bool? AutoSavepointsEnabled { get; }
}
}
93 changes: 93 additions & 0 deletions test/EFCore.Relational.Specification.Tests/TransactionTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
using Xunit;
using IsolationLevel = System.Data.IsolationLevel;

// ReSharper disable MethodHasAsyncOverload
// ReSharper disable InconsistentNaming
namespace Microsoft.EntityFrameworkCore
{
Expand Down Expand Up @@ -1209,6 +1210,98 @@ public virtual async Task Externally_closed_connections_are_handled_correctly(bo
Assert.Equal(ConnectionState.Closed, connection.State);
}

[ConditionalTheory]
[InlineData(true)]
[InlineData(false)]
public virtual async Task SaveChanges_implicitly_creates_savepoint(bool async)
{
using (var context = CreateContext())
{
Assert.True(context.Database.AutoSavepointsEnabled);

using var transaction = async
? await context.Database.BeginTransactionAsync()
: context.Database.BeginTransaction();

context.Add(new TransactionCustomer { Id = 77, Name = "Bobble" });

if (async)
{
await context.SaveChangesAsync();
}
else
{
context.SaveChanges();
}

context.Add(new TransactionCustomer { Id = 78, Name = "Hobble" });
context.Add(new TransactionCustomer { Id = 1, Name = "Gobble" }); // Cause SaveChanges failure

if (async)
{
await Assert.ThrowsAsync<DbUpdateException>(() => context.SaveChangesAsync());
await transaction.CommitAsync();
}
else
{
Assert.Throws<DbUpdateException>(() => context.SaveChanges());
transaction.Commit();
}
}

using (var context = CreateContext())
{
Assert.Equal(77, context.Set<TransactionCustomer>().Max(c => c.Id));
}
}

[ConditionalTheory]
[InlineData(true)]
[InlineData(false)]
public virtual async Task SaveChanges_can_be_used_with_no_savepoint(bool async)
{
using (var context = CreateContext())
{
context.Database.AutoSavepointsEnabled = false;

using var transaction = async
? await context.Database.BeginTransactionAsync()
: context.Database.BeginTransaction();

context.Add(new TransactionCustomer { Id = 77, Name = "Bobble" });

if (async)
{
await context.SaveChangesAsync();
}
else
{
context.SaveChanges();
}

context.Add(new TransactionCustomer { Id = 78, Name = "Hobble" });
context.Add(new TransactionCustomer { Id = 1, Name = "Gobble" }); // Cause SaveChanges failure

if (async)
{
await Assert.ThrowsAsync<DbUpdateException>(() => context.SaveChangesAsync());
await transaction.CommitAsync();
}
else
{
Assert.Throws<DbUpdateException>(() => context.SaveChanges());
transaction.Commit();
}

context.Database.AutoSavepointsEnabled = true;
}

using (var context = CreateContext())
{
Assert.Equal(78, context.Set<TransactionCustomer>().Max(c => c.Id));
}
}

[ConditionalTheory]
[InlineData(true)]
[InlineData(false)]
Expand Down
11 changes: 11 additions & 0 deletions test/EFCore.SqlServer.FunctionalTests/DbContextPoolingTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ public PooledContext(DbContextOptions options)
ChangeTracker.AutoDetectChangesEnabled = false;
ChangeTracker.LazyLoadingEnabled = false;
Database.AutoTransactionsEnabled = false;
Database.AutoSavepointsEnabled = false;
ChangeTracker.CascadeDeleteTiming = CascadeTiming.Never;
ChangeTracker.DeleteOrphansTiming = CascadeTiming.Never;
}
Expand Down Expand Up @@ -539,6 +540,7 @@ public async Task Context_configuration_is_reset(bool useInterface, bool async)
context1.ChangeTracker.CascadeDeleteTiming = CascadeTiming.Immediate;
context1.ChangeTracker.DeleteOrphansTiming = CascadeTiming.Immediate;
context1.Database.AutoTransactionsEnabled = true;
context1.Database.AutoSavepointsEnabled = true;
context1.SavingChanges += (sender, args) => { };
context1.SavedChanges += (sender, args) => { };
context1.SaveChangesFailed += (sender, args) => { };
Expand Down Expand Up @@ -568,6 +570,7 @@ public async Task Context_configuration_is_reset(bool useInterface, bool async)
Assert.Equal(CascadeTiming.Never, context2.ChangeTracker.CascadeDeleteTiming);
Assert.Equal(CascadeTiming.Never, context2.ChangeTracker.DeleteOrphansTiming);
Assert.False(context2.Database.AutoTransactionsEnabled);
Assert.False(context2.Database.AutoSavepointsEnabled);
}

[ConditionalTheory]
Expand All @@ -585,6 +588,7 @@ public async Task Context_configuration_is_reset_with_factory(bool async)
context1.ChangeTracker.CascadeDeleteTiming = CascadeTiming.Immediate;
context1.ChangeTracker.DeleteOrphansTiming = CascadeTiming.Immediate;
context1.Database.AutoTransactionsEnabled = true;
context1.Database.AutoSavepointsEnabled = true;
context1.SavingChanges += (sender, args) => { };
context1.SavedChanges += (sender, args) => { };
context1.SaveChangesFailed += (sender, args) => { };
Expand All @@ -609,6 +613,7 @@ public async Task Context_configuration_is_reset_with_factory(bool async)
Assert.Equal(CascadeTiming.Never, context2.ChangeTracker.CascadeDeleteTiming);
Assert.Equal(CascadeTiming.Never, context2.ChangeTracker.DeleteOrphansTiming);
Assert.False(context2.Database.AutoTransactionsEnabled);
Assert.False(context2.Database.AutoSavepointsEnabled);
}

[ConditionalFact]
Expand All @@ -628,6 +633,7 @@ public void Change_tracker_can_be_cleared_without_resetting_context_config()
context.ChangeTracker.CascadeDeleteTiming = CascadeTiming.Immediate;
context.ChangeTracker.DeleteOrphansTiming = CascadeTiming.Immediate;
context.Database.AutoTransactionsEnabled = true;
context.Database.AutoSavepointsEnabled = true;
context.ChangeTracker.Tracked += ChangeTracker_OnTracked;
context.ChangeTracker.StateChanged += ChangeTracker_OnStateChanged;
context.SavingChanges += (sender, args) => { };
Expand All @@ -642,6 +648,7 @@ public void Change_tracker_can_be_cleared_without_resetting_context_config()
Assert.Equal(CascadeTiming.Immediate, context.ChangeTracker.CascadeDeleteTiming);
Assert.Equal(CascadeTiming.Immediate, context.ChangeTracker.DeleteOrphansTiming);
Assert.True(context.Database.AutoTransactionsEnabled);
Assert.True(context.Database.AutoSavepointsEnabled);

Assert.False(_changeTracker_OnTracked);
Assert.False(_changeTracker_OnStateChanged);
Expand Down Expand Up @@ -687,6 +694,7 @@ public async Task Default_Context_configuration_is_reset(bool async)
context1.ChangeTracker.LazyLoadingEnabled = false;
context1.ChangeTracker.QueryTrackingBehavior = QueryTrackingBehavior.NoTracking;
context1.Database.AutoTransactionsEnabled = false;
context1.Database.AutoSavepointsEnabled = false;
context1.ChangeTracker.CascadeDeleteTiming = CascadeTiming.Immediate;
context1.ChangeTracker.DeleteOrphansTiming = CascadeTiming.Immediate;

Expand All @@ -705,6 +713,7 @@ public async Task Default_Context_configuration_is_reset(bool async)
Assert.Equal(CascadeTiming.Immediate, context2.ChangeTracker.CascadeDeleteTiming);
Assert.Equal(CascadeTiming.Immediate, context2.ChangeTracker.DeleteOrphansTiming);
Assert.True(context2.Database.AutoTransactionsEnabled);
Assert.True(context2.Database.AutoSavepointsEnabled);
}

[ConditionalTheory]
Expand All @@ -721,6 +730,7 @@ public async Task Default_Context_configuration_is_reset_with_factory(bool async
context1.ChangeTracker.LazyLoadingEnabled = false;
context1.ChangeTracker.QueryTrackingBehavior = QueryTrackingBehavior.NoTracking;
context1.Database.AutoTransactionsEnabled = false;
context1.Database.AutoSavepointsEnabled = false;
context1.ChangeTracker.CascadeDeleteTiming = CascadeTiming.Immediate;
context1.ChangeTracker.DeleteOrphansTiming = CascadeTiming.Immediate;

Expand All @@ -736,6 +746,7 @@ public async Task Default_Context_configuration_is_reset_with_factory(bool async
Assert.Equal(CascadeTiming.Immediate, context2.ChangeTracker.CascadeDeleteTiming);
Assert.Equal(CascadeTiming.Immediate, context2.ChangeTracker.DeleteOrphansTiming);
Assert.True(context2.Database.AutoTransactionsEnabled);
Assert.True(context2.Database.AutoSavepointsEnabled);
}

[ConditionalTheory]
Expand Down
18 changes: 12 additions & 6 deletions test/EFCore.SqlServer.FunctionalTests/TransactionSqlServerTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,22 @@ public TransactionSqlServerTest(TransactionSqlServerFixture fixture)
{
}

// Test relies on savepoints, which are disabled when MARS is enabled
public override Task SaveChanges_implicitly_creates_savepoint(bool async)
=> new SqlConnectionStringBuilder(TestStore.ConnectionString).MultipleActiveResultSets
? Task.CompletedTask
: base.SaveChanges_implicitly_creates_savepoint(async);

// Savepoints cannot be released in SQL Server
public override Task Savepoint_can_be_released(bool async)
=> Task.CompletedTask;

// Test relies on savepoints, which are disabled when MARS is enabled
public override Task SaveChanges_uses_explicit_transaction_with_failure_behavior(bool async, bool autoTransaction)
=> new SqlConnectionStringBuilder(TestStore.ConnectionString).MultipleActiveResultSets
? Task.CompletedTask
: base.SaveChanges_uses_explicit_transaction_with_failure_behavior(async, autoTransaction);

[ConditionalTheory]
[InlineData(true)]
[InlineData(false)]
Expand Down Expand Up @@ -53,12 +65,6 @@ public virtual async Task Savepoints_are_disabled_with_MARS(bool async)
Assert.Contains(Fixture.ListLoggerFactory.Log, t => t.Id == SqlServerEventId.SavepointsDisabledBecauseOfMARS);
}

// Test relies on savepoints, which are disabled when MARS is enabled
public override Task SaveChanges_uses_explicit_transaction_with_failure_behavior(bool async, bool autoTransaction)
=> new SqlConnectionStringBuilder(TestStore.ConnectionString).MultipleActiveResultSets
? Task.CompletedTask
: base.SaveChanges_uses_explicit_transaction_with_failure_behavior(async, autoTransaction);

protected override bool SnapshotSupported
=> true;

Expand Down