Skip to content

Make IQueryExpressionInterceptor an ISingletonInterceptor #36128

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
May 28, 2025
Merged

Make IQueryExpressionInterceptor an ISingletonInterceptor #36128

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion src/EFCore/Diagnostics/IQueryExpressionInterceptor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ namespace Microsoft.EntityFrameworkCore.Diagnostics;
/// See <see href="https://aka.ms/efcore-docs-interceptors">EF Core interceptors</see> for more information and examples.
/// </para>
/// </remarks>
public interface IQueryExpressionInterceptor : IInterceptor
public interface IQueryExpressionInterceptor : ISingletonInterceptor
{
/// <summary>
/// Called with the LINQ expression tree for a query before it is compiled.
Expand Down
15 changes: 9 additions & 6 deletions test/EFCore.Cosmos.FunctionalTests/QueryExpressionInterceptionWithDiagnosticsCosmosTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,17 @@ public class QueryExpressionInterceptionWithDiagnosticsCosmosTest(
: QueryExpressionInterceptionTestBase(fixture),
IClassFixture<QueryExpressionInterceptionWithDiagnosticsCosmosTest.InterceptionCosmosFixture>
{
public override Task Intercept_query_passively(bool async, bool inject)
=> CosmosTestHelpers.Instance.NoSyncTest(async, a => base.Intercept_query_passively(a, inject));
public override Task Intercept_query_passively(bool async)
=> CosmosTestHelpers.Instance.NoSyncTest(async, a => base.Intercept_query_passively(a));

public override Task Intercept_query_with_multiple_interceptors(bool async, bool inject)
=> CosmosTestHelpers.Instance.NoSyncTest(async, a => base.Intercept_query_with_multiple_interceptors(a, inject));
public override Task Intercept_query_with_multiple_interceptors(bool async)
=> CosmosTestHelpers.Instance.NoSyncTest(async, a => base.Intercept_query_with_multiple_interceptors(a));

public override Task Intercept_to_change_query_expression(bool async, bool inject)
=> CosmosTestHelpers.Instance.NoSyncTest(async, a => base.Intercept_to_change_query_expression(a, inject));
public override Task Intercept_to_change_query_expression(bool async)
=> CosmosTestHelpers.Instance.NoSyncTest(async, a => base.Intercept_to_change_query_expression(a));

public override Task Interceptor_does_not_leak_across_contexts(bool async)
=> CosmosTestHelpers.Instance.NoSyncTest(async, a => base.Interceptor_does_not_leak_across_contexts(a));

public class InterceptionCosmosFixture : InterceptionFixtureBase
{
Expand Down
3 changes: 3 additions & 0 deletions test/EFCore.InMemory.FunctionalTests/QueryExpressionInterceptionInMemoryTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ public abstract class QueryExpressionInterceptionInMemoryTestBase(
QueryExpressionInterceptionInMemoryTestBase.InterceptionInMemoryFixtureBase fixture)
: QueryExpressionInterceptionTestBase(fixture)
{
public override Task Interceptor_does_not_leak_across_contexts(bool async)
=> Task.CompletedTask;

public override async Task<UniverseContext> SeedAsync(UniverseContext context)
{
await base.SeedAsync(context);
Expand Down
81 changes: 47 additions & 34 deletions test/EFCore.Specification.Tests/QueryExpressionInterceptionTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,55 +9,43 @@ public abstract class QueryExpressionInterceptionTestBase(InterceptionTestBase.I
: InterceptionTestBase(fixture)
{
[ConditionalTheory]
[InlineData(false, false)]
[InlineData(true, false)]
[InlineData(false, true)]
[InlineData(true, true)]
public virtual async Task Intercept_query_passively(bool async, bool inject)
[MemberData(nameof(IsAsyncData))]
Copy link
Member Author

Choose a reason for hiding this comment

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

@AndriySvyryd note that now that the interceptor is a singleton, we can't test "app interceptor" setup since the tests use an externally-provided internal service provider. It seems like the general interceptor machinery is already well-covered via other non-singleton interceptors that we can not also exercise it here.

public virtual async Task Intercept_query_passively(bool async)
{
var (context, interceptor) = await CreateContextAsync<TestQueryExpressionInterceptor>(inject);
var (context, interceptor) = await CreateContextAsync<TestQueryExpressionInterceptor>(inject: true);

using var _ = context;

var query = context.Set<Singularity>().Where(e => e.Type == "Black Hole");
var results = async ? await query.ToListAsync() : query.ToList();
var result = async ? await query.SingleAsync() : query.Single();

Assert.Single(results);
Assert.Equal("Black Hole", results[0].Type);
Assert.Equal("Black Hole", result.Type);

AssertNormalOutcome(context, interceptor);

Assert.Contains(@".Where(e => e.Type == ""Black Hole"")", interceptor.QueryExpression);
Assert.Contains(""".Where(e => e.Type == "Black Hole")""", interceptor.QueryExpression);
}

[ConditionalTheory]
[InlineData(false, false)]
[InlineData(true, false)]
[InlineData(false, true)]
[InlineData(true, true)]
public virtual async Task Intercept_query_with_multiple_interceptors(bool async, bool inject)
[MemberData(nameof(IsAsyncData))]
public virtual async Task Intercept_query_with_multiple_interceptors(bool async)
{
var interceptor1 = new TestQueryExpressionInterceptor();
var interceptor2 = new QueryChangingExpressionInterceptor();
var interceptor3 = new TestQueryExpressionInterceptor();
var interceptor4 = new TestQueryExpressionInterceptor();

using var context = await CreateContextAsync(
new IInterceptor[] { new TestQueryExpressionInterceptor(), interceptor1, interceptor2 },
new IInterceptor[] { interceptor3, interceptor4, new TestQueryExpressionInterceptor() });
appInterceptor: null,
[interceptor1, interceptor2]);

using var listener = Fixture.SubscribeToDiagnosticListener(context.ContextId);

var query = context.Set<Singularity>().Where(e => e.Type == "Bing Bang");
var results = async ? await query.ToListAsync() : query.ToList();
var result = async ? await query.SingleAsync() : query.Single();

Assert.Single(results);
Assert.Equal("Bing Bang", results[0].Type);
Assert.Equal("Bing Bang", result.Type);

AssertNormalOutcome(context, interceptor1);
AssertNormalOutcome(context, interceptor2);
AssertNormalOutcome(context, interceptor3);
AssertNormalOutcome(context, interceptor4);

listener.AssertEventsInOrder(
CoreEventId.QueryCompilationStarting.Name,
Expand All @@ -67,25 +55,48 @@ public virtual async Task Intercept_query_with_multiple_interceptors(bool async,
}

[ConditionalTheory]
[InlineData(false, false)]
[InlineData(true, false)]
[InlineData(false, true)]
[InlineData(true, true)]
public virtual async Task Intercept_to_change_query_expression(bool async, bool inject)
[MemberData(nameof(IsAsyncData))]
public virtual async Task Intercept_to_change_query_expression(bool async)
{
var (context, interceptor) = await CreateContextAsync<QueryChangingExpressionInterceptor>(inject);
var (context, interceptor) = await CreateContextAsync<QueryChangingExpressionInterceptor>(inject: true);

using var _ = context;

var query = context.Set<Singularity>().Where(e => e.Type == "Black Hole");
var results = async ? await query.ToListAsync() : query.ToList();
var result = async ? await query.SingleAsync() : query.Single();

Assert.Single(results);
Assert.Equal("Bing Bang", results[0].Type);
Assert.Equal("Bing Bang", result.Type);

AssertNormalOutcome(context, interceptor);

Assert.Contains(@".Where(e => e.Type == ""Bing Bang"")", interceptor.QueryExpression);
Assert.Contains(""".Where(e => e.Type == "Bing Bang")""", interceptor.QueryExpression);
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual async Task Interceptor_does_not_leak_across_contexts(bool async)
{
// Create one context with QueryChangingExpressionInterceptor, and another with TestQueryExpressionInterceptor (which is a no-op).
// Note that we don't use the regular suite infra for creating the contexts, as that creates separate service providers for each
// one, but that's exactly what we want to test here.
using var context1 = new UniverseContext(
Fixture.AddOptions(
Fixture.TestStore.AddProviderOptions(
new DbContextOptionsBuilder<DbContext>().AddInterceptors(new QueryChangingExpressionInterceptor())))
.Options);
using var context2 = new UniverseContext(
Fixture.AddOptions(
Fixture.TestStore.AddProviderOptions(
new DbContextOptionsBuilder<DbContext>().AddInterceptors(new TestQueryExpressionInterceptor())))
.Options);

var query1 = context1.Set<Singularity>().Where(e => e.Type == "Black Hole");
var result1 = async ? await query1.SingleAsync() : query1.Single();
Assert.Equal("Bing Bang", result1.Type);

var query2 = context2.Set<Singularity>().Where(e => e.Type == "Black Hole");
var result2 = async ? await query2.SingleAsync() : query2.Single();
Assert.Equal("Black Hole", result2.Type);
}

protected class QueryChangingExpressionInterceptor : TestQueryExpressionInterceptor
Expand Down Expand Up @@ -128,4 +139,6 @@ public virtual Expression QueryCompilationStarting(
return queryExpression;
}
}

public static readonly IEnumerable<object[]> IsAsyncData = [[false], [true]];
}