Make IQueryExpressionInterceptor an ISingletonService

[GianvitoDifilippo]

Hello everyone,
I implemented an IQueryExpressionInterceptor for multitenancy, which modifies the expression by adding a Where filter like so:

DbContext
  .Set<MyEntity>()
  .Where(entity => entity.TenantId == myTenantId) // <- This is added by the interceptor
  .FirstOrDefault(entity => entity.Id == myId);

The expression entity => entity.TenantId == myTenantId is constructed dynamically by the interceptor kind of like this (simplified):

ParameterExpression entity = Expression.Parameter(typeof(MyEntity));
LambdaExpression predicate = Expression.Lambda(
  body: Expression.Equal(
    left: Expression.Property(
      expression: entity,
      propertyName: "TenantId"),
    right: Expression.Constant(myTenantId)), // I figured this was the only way since I couldn't find a way to inject parameters into the QueryContext
  parameters: new[] { entity });

Obviously, the value of 'myTenantId' comes from a scoped service which reads the current user's claims.
The problem is that this doesn't work, since the QueryCompiler service caches the compiled query using the original query expression instead of the new one. This means all subsequent calls to FirstOrDefault(entity => entity.Id == myId) will use the tenant id used in the first interception.

Is this intended behavior? If yes, I would expect IQueryExpressionInterceptor to implement ISingletonInterceptor instead of IInterceptor - if my understanding of these interfaces is correct - since the interception result will be permanently cached.

Anyway, is there a way to achieve the desired behavior with this new interceptor? I intended to override EntityQueryProvider only as a last resort :)
Note: I am aware of the HasQueryFilter method, I'm not using it because of more complex logic that was omitted in this example.

Tested with SqlServer and Postgres, attaching example code using InMemory.

QueryExpressionInterceptorIssue.zip

[ajcvickers]

@GianvitoDifilippo As you have found, queries are cached, so this won't work.

Is this intended behavior? If yes, I would expect IQueryExpressionInterceptor to implement ISingletonInterceptor instead of IInterceptor - if my understanding of these interfaces is correct - since the interception result will be permanently cached.

This is a good point. We will discuss.

[roji]

Note: I am aware of the HasQueryFilter method, I'm not using it because of more complex logic that was omitted in this example.

No problem, but it would be good to know exactly what limitations of query filters made you look at the new query experssion interceptor (as we may want to improve query filters in the future).

[GianvitoDifilippo]

First of all, thank you all for your answers.

The main reason I don't think I can use HasQueryFilter for this use case is that I may have multiple query filters (soft delete, etc.) and would like to be able to turn them on and off separately, both

1. in the query definition with an extension method, e.g., .AsNoMultitenancy(), AsNoSoftDelete(), similarly to what proposed in this issue and
2. using a flag defined on the scoped service that also provides the parameters for the query.

I actually did manage to achieve (2) using this solution, as we can see in this log,

Generated query execution expression

      'queryContext =>
      {
          queryContext.AddParameter(
              name: "__ef_filter__p_0",
              value: (object)Invoke(queryContext => (object)!((MyDbContext)queryContext.Context
                  .GetService().ShouldFilter), queryContext));
          queryContext.AddParameter(
              name: "__ef_filter__TenantId_1",
              value: (object)Invoke(queryContext => (object)(MyDbContext)queryContext.Context
                  .GetService().TenantId, queryContext));
          return ShapedQueryCompilingExpressionVisitor.SingleOrDefaultAsync<MyEntity>(
              asyncEnumerable: new SingleQueryingEnumerable<MyEntity>(
                  (RelationalQueryContext)queryContext,
                  RelationalCommandCache.QueryExpression(
                      Projection Mapping:
                          EmptyProjectionMember -> Dictionary<IProperty, int> { [Property: MyEntity.Id (Guid) Required PK AfterSave:Throw ValueGenerated.OnAdd, 0], [Property: MyEntity.TenantId (Guid) Required, 1] }
                      SELECT TOP(1) m.Id, m.TenantId
                      FROM MyEntity AS m
                      WHERE (@__ef_filter__p_0 || (m.TenantId == @__ef_filter__TenantId_1)) && (m.Id == @__id_0)),
                  null,
                  Func<QueryContext, DbDataReader, ResultContext, SingleQueryResultCoordinator, MyEntity>,
                  MyDbContext,
                  False,
                  False,
                  True
              ),
              cancellationToken: queryContext.CancellationToken);

This is totally fine, but since this will generate something like this WHERE (@__ef_filter__p_0 = CAST(1 AS bit) OR [m].[TenantId] = @__ef_filter__TenantId_1), and I wanted to provide also (1), I thought the new interceptor could be used to achieve both points and generate cleaner SQL.

The other reason why I wanted to use it is I could inject expressions that are not filters in the query - which I think is its original purpose - but using data provided by scoped services and providing points (1) and (2).

Point (2) is actually more important than point (1), so I will evaluate both solutions (the other being overriding EntityQueryProvider).