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

Reset query tracking behavior correctly if set on DbContextOptions #29766

Merged
merged 1 commit into from
Dec 8, 2022
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
8 changes: 6 additions & 2 deletions src/EFCore/DbContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -882,8 +882,12 @@ private void SetLeaseInternal(DbContextLease lease)
|| _configurationSnapshot.HasChangeTrackerConfiguration)
{
var changeTracker = ChangeTracker;
((IResettableService)changeTracker).ResetState();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we're calling ResetState here just in order to set _queryTrackingBehavior back to _defaultQueryTrackingBehavior, in case we don't need to set it here again? Because it also sets all the other stuff that we'll just overwrite here below. Any way for us to just capture the default in the configuration snapshot so we can always just write it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried that first, but it gets complicated because at the time we create the snapshot we don't necessarily have services available to find the default, and likewise, the ChangeTracker itself may not be available. So things broke.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I just get sad when we add more stuff to the context resetting...

changeTracker.AutoDetectChangesEnabled = _configurationSnapshot.AutoDetectChangesEnabled;
changeTracker.QueryTrackingBehavior = _configurationSnapshot.QueryTrackingBehavior;
if (_configurationSnapshot.QueryTrackingBehavior.HasValue)
{
changeTracker.QueryTrackingBehavior = _configurationSnapshot.QueryTrackingBehavior.Value;
}
changeTracker.LazyLoadingEnabled = _configurationSnapshot.LazyLoadingEnabled;
changeTracker.CascadeDeleteTiming = _configurationSnapshot.CascadeDeleteTiming;
changeTracker.DeleteOrphansTiming = _configurationSnapshot.DeleteOrphansTiming;
Expand Down Expand Up @@ -940,7 +944,7 @@ void IDbContextPoolable.SnapshotConfiguration()
_changeTracker != null,
changeDetectorEvents != null,
_changeTracker?.AutoDetectChangesEnabled ?? true,
_changeTracker?.QueryTrackingBehavior ?? QueryTrackingBehavior.TrackAll,
_changeTracker?.QueryTrackingBehavior,
_database?.AutoTransactionBehavior ?? AutoTransactionBehavior.WhenNeeded,
_database?.AutoSavepointsEnabled ?? true,
_changeTracker?.LazyLoadingEnabled ?? true,
Expand Down
4 changes: 2 additions & 2 deletions src/EFCore/Internal/DbContextPoolConfigurationSnapshot.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public DbContextPoolConfigurationSnapshot(
bool hasChangeTrackerConfiguration,
bool hasChangeDetectorConfiguration,
bool autoDetectChangesEnabled,
QueryTrackingBehavior queryTrackingBehavior,
QueryTrackingBehavior? queryTrackingBehavior,
AutoTransactionBehavior autoTransactionBehavior,
bool autoSavepointsEnabled,
bool lazyLoadingEnabled,
Expand Down Expand Up @@ -135,7 +135,7 @@ public DbContextPoolConfigurationSnapshot(
/// 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 QueryTrackingBehavior QueryTrackingBehavior { get; }
public QueryTrackingBehavior? QueryTrackingBehavior { get; }

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down
112 changes: 88 additions & 24 deletions test/EFCore.SqlServer.FunctionalTests/DbContextPoolingTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,51 @@ private static DbContextOptionsBuilder ConfigureOptions(DbContextOptionsBuilder
.UseSqlServer(SqlServerNorthwindTestStoreFactory.NorthwindConnectionString)
.EnableServiceProviderCaching(false);

private static IServiceProvider BuildServiceProvider<TContextService, TContext>()
private static IServiceProvider BuildServiceProvider<TContextService, TContext>(Action<DbContextOptionsBuilder> optionsAction = null)
where TContextService : class
where TContext : DbContext, TContextService
=> new ServiceCollection()
.AddDbContextPool<TContextService, TContext>(ob => ConfigureOptions(ob))
.AddDbContextPool<ISecondContext, SecondContext>(ob => ConfigureOptions(ob))
.AddDbContextPool<TContextService, TContext>(
ob =>
{
var builder = ConfigureOptions(ob);
if (optionsAction != null)
{
optionsAction(builder);
}
})
.AddDbContextPool<ISecondContext, SecondContext>(
ob =>
{
var builder = ConfigureOptions(ob);
if (optionsAction != null)
{
optionsAction(builder);
}
})
.BuildServiceProvider(validateScopes: true);

private static IServiceProvider BuildServiceProvider<TContext>()
private static IServiceProvider BuildServiceProvider<TContext>(Action<DbContextOptionsBuilder> optionsAction = null)
where TContext : DbContext
=> new ServiceCollection()
.AddDbContextPool<TContext>(ob => ConfigureOptions(ob))
.AddDbContextPool<SecondContext>(ob => ConfigureOptions(ob))
.AddDbContextPool<TContext>(
ob =>
{
var builder = ConfigureOptions(ob);
if (optionsAction != null)
{
optionsAction(builder);
}
})
.AddDbContextPool<SecondContext>(
ob =>
{
var builder = ConfigureOptions(ob);
if (optionsAction != null)
{
optionsAction(builder);
}
})
.BuildServiceProvider(validateScopes: true);

private static IServiceProvider BuildServiceProviderWithFactory<TContext>()
Expand Down Expand Up @@ -758,15 +790,27 @@ public async Task Contexts_are_pooled_with_factory(bool async, bool withDependen
}

[ConditionalTheory]
[InlineData(false, false)]
[InlineData(true, false)]
[InlineData(false, true)]
[InlineData(true, true)]
public async Task Context_configuration_is_reset(bool useInterface, bool async)
[InlineData(false, false, null)]
[InlineData(true, false, null)]
[InlineData(false, true, null)]
[InlineData(true, true, null)]
[InlineData(false, false, QueryTrackingBehavior.TrackAll)]
[InlineData(true, false, QueryTrackingBehavior.TrackAll)]
[InlineData(false, true, QueryTrackingBehavior.TrackAll)]
[InlineData(true, true, QueryTrackingBehavior.TrackAll)]
[InlineData(false, false, QueryTrackingBehavior.NoTracking)]
[InlineData(true, false, QueryTrackingBehavior.NoTracking)]
[InlineData(false, true, QueryTrackingBehavior.NoTracking)]
[InlineData(true, true, QueryTrackingBehavior.NoTracking)]
[InlineData(false, false, QueryTrackingBehavior.NoTrackingWithIdentityResolution)]
[InlineData(true, false, QueryTrackingBehavior.NoTrackingWithIdentityResolution)]
[InlineData(false, true, QueryTrackingBehavior.NoTrackingWithIdentityResolution)]
[InlineData(true, true, QueryTrackingBehavior.NoTrackingWithIdentityResolution)]
public async Task Context_configuration_is_reset(bool useInterface, bool async, QueryTrackingBehavior? queryTrackingBehavior)
{
var serviceProvider = useInterface
? BuildServiceProvider<IPooledContext, PooledContext>()
: BuildServiceProvider<PooledContext>();
? BuildServiceProvider<IPooledContext, PooledContext>(b => UseQueryTrackingBehavior(b, queryTrackingBehavior))
: BuildServiceProvider<PooledContext>(b => UseQueryTrackingBehavior(b, queryTrackingBehavior));

var serviceScope = serviceProvider.CreateScope();
var scopedProvider = serviceScope.ServiceProvider;
Expand Down Expand Up @@ -828,7 +872,7 @@ public async Task Context_configuration_is_reset(bool useInterface, bool async)

Assert.False(context2!.ChangeTracker.AutoDetectChangesEnabled);
Assert.False(context2.ChangeTracker.LazyLoadingEnabled);
Assert.Equal(QueryTrackingBehavior.TrackAll, context2.ChangeTracker.QueryTrackingBehavior);
Assert.Equal(queryTrackingBehavior ?? QueryTrackingBehavior.TrackAll, context2.ChangeTracker.QueryTrackingBehavior);
Assert.Equal(CascadeTiming.Never, context2.ChangeTracker.CascadeDeleteTiming);
Assert.Equal(CascadeTiming.Never, context2.ChangeTracker.DeleteOrphansTiming);
Assert.Equal(AutoTransactionBehavior.Never, context2.Database.AutoTransactionBehavior);
Expand Down Expand Up @@ -869,11 +913,17 @@ public async Task Context_configuration_is_reset(bool useInterface, bool async)
}

[ConditionalTheory]
[InlineData(false)]
[InlineData(true)]
public async Task Uninitialized_context_configuration_is_reset_properly(bool async)
[InlineData(false, null)]
[InlineData(true, null)]
[InlineData(false, QueryTrackingBehavior.TrackAll)]
[InlineData(true, QueryTrackingBehavior.TrackAll)]
[InlineData(false, QueryTrackingBehavior.NoTracking)]
[InlineData(true, QueryTrackingBehavior.NoTracking)]
[InlineData(false, QueryTrackingBehavior.NoTrackingWithIdentityResolution)]
[InlineData(true, QueryTrackingBehavior.NoTrackingWithIdentityResolution)]
public async Task Uninitialized_context_configuration_is_reset_properly(bool async, QueryTrackingBehavior? queryTrackingBehavior)
{
var serviceProvider = BuildServiceProvider<SecondContext>();
var serviceProvider = BuildServiceProvider<SecondContext>(b => UseQueryTrackingBehavior(b, queryTrackingBehavior));

var serviceScope = serviceProvider.CreateScope();
var ctx = serviceScope.ServiceProvider.GetRequiredService<SecondContext>();
Expand Down Expand Up @@ -1122,11 +1172,17 @@ private void ChangeTracker_OnDetectedEntityChanges(object sender, DetectedEntity
=> _changeTracker_OnDetectedEntityChanges = true;

[ConditionalTheory]
[InlineData(false)]
[InlineData(true)]
public async Task Default_Context_configuration_is_reset(bool async)
[InlineData(false, null)]
[InlineData(true, null)]
[InlineData(false, QueryTrackingBehavior.TrackAll)]
[InlineData(true, QueryTrackingBehavior.TrackAll)]
[InlineData(false, QueryTrackingBehavior.NoTracking)]
[InlineData(true, QueryTrackingBehavior.NoTracking)]
[InlineData(false, QueryTrackingBehavior.NoTrackingWithIdentityResolution)]
[InlineData(true, QueryTrackingBehavior.NoTrackingWithIdentityResolution)]
public async Task Default_Context_configuration_is_reset(bool async, QueryTrackingBehavior? queryTrackingBehavior)
{
var serviceProvider = BuildServiceProvider<DefaultOptionsPooledContext>();
var serviceProvider = BuildServiceProvider<DefaultOptionsPooledContext>(b => UseQueryTrackingBehavior(b, queryTrackingBehavior));

var serviceScope = serviceProvider.CreateScope();
var scopedProvider = serviceScope.ServiceProvider;
Expand All @@ -1152,7 +1208,7 @@ public async Task Default_Context_configuration_is_reset(bool async)

Assert.True(context2!.ChangeTracker.AutoDetectChangesEnabled);
Assert.True(context2.ChangeTracker.LazyLoadingEnabled);
Assert.Equal(QueryTrackingBehavior.TrackAll, context2.ChangeTracker.QueryTrackingBehavior);
Assert.Equal(queryTrackingBehavior ?? QueryTrackingBehavior.TrackAll, context2.ChangeTracker.QueryTrackingBehavior);
Assert.Equal(CascadeTiming.Immediate, context2.ChangeTracker.CascadeDeleteTiming);
Assert.Equal(CascadeTiming.Immediate, context2.ChangeTracker.DeleteOrphansTiming);
Assert.Equal(AutoTransactionBehavior.WhenNeeded, context2.Database.AutoTransactionBehavior);
Expand Down Expand Up @@ -1441,7 +1497,7 @@ public async Task Provider_services_are_reset(bool useInterface, bool async)
{
var serviceProvider = useInterface
? BuildServiceProvider<IPooledContext, PooledContext>()
: BuildServiceProvider<PooledContext>();
: BuildServiceProvider<PooledContext>(o => o.UseQueryTrackingBehavior(QueryTrackingBehavior.NoTracking));

var serviceScope = serviceProvider.CreateScope();
var scopedProvider = serviceScope.ServiceProvider;
Expand Down Expand Up @@ -1968,6 +2024,14 @@ await Task.WhenAll(
})));
}

private void UseQueryTrackingBehavior(DbContextOptionsBuilder optionsBuilder, QueryTrackingBehavior? queryTrackingBehavior)
{
if (queryTrackingBehavior.HasValue)
{
optionsBuilder.UseQueryTrackingBehavior(queryTrackingBehavior.Value);
}
}

private async Task Dispose(IDisposable disposable, bool async)
{
if (async)
Expand Down