Make parameterization of global query filters more obvious

[AndersMalmgren]

From below:

Notes from triage: the current design of query filters uses the DbContext as the anchor from which filter parameters are read. This works well in many cases, but has a couple of limitations:

• People don't get it. That is, many times we see code that attempts to hard-code the filter value into the model and then re-build the model for each context instance. This can be made to work, but only with terrible perf characteristics, which makes it a pit-of-failure.
• It requires a stronger coupling of the DbContext type to the entity types being used than is ideal--essentially the issue reported above.

Based on this, it might be worth designing and implementing query filters where the filter parameter is obtained in a different way--such as through a defined callback. This would be easier for many to understand and would not have a strong coupling to the context.

---

Original issue:

For a enterprise level application you cant define an entire data model directly on the DbContext, you use IEntityTypeConfiguration<TEntity> to seperate configuration away from OnModelCreating method.

When it comes to HasQueryFilter that uses expressions with dependencies to scoped members there is no such mechanics. It needs to be defined directly on the DbContext. This just wont cut it for a large Enterprise level application.

[smitpatel]

Your assumption is wrong. There are various different ways in which you can define Query filter outside of DbContext and still use it to change value of parameters.
Out tests here covers a large pattern which is supported.

https://github.com/aspnet/EntityFrameworkCore/blob/37de58b3addf44e83fe0fefbd5be13c39bed3357/test/EFCore.Specification.Tests/TestModels/QueryFilterFuncletizationContext.cs#L33-L60

[AndersMalmgren]

But the fields are defined on DbContext, the fields need to be defined on the custom class for true seperation. For examaple. I want todo something like this

public class AgreementAuthorization : IEntityAuthorization
{
    private readonly string _legalEntityNumber;

    public AgreementAuthorization(IAuthScope scope) //Scoped through scoped service provider
    {
        _legalEntityNumber = scope.LegalEntityNumber; 
    }

    public void Build(ModelBuilder builder)
    {
        builder
            .Entity<Agreement>()
            .HasQueryFilter(a => _legalEntityNumber == null || a.LegalEntity.Number == _legalEntityNumber);
    }
}

public class MyDbContext : DbContext 
{
   private IEnumerable<IEntityAuthorization> _entityAuthorization;

   MyDbContext(IEnumerable<IEntityAuthorization> entityAuthorization) 
   {
      _entityAuthorization = entityAuthorization;
   }

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {                                
        builder.ApplyConfigurationsFromAssembly(typeof(MyDbContext).Assembly);

        _entityAuthorization.ForEach(a => a.Build(builder));
    }
}

[smitpatel]

First you are defining QueryFilter for same EntityType repeatedly so that will be overwritten.

Even if I ignore that, can you describe how do you want us to figure out exact value to use at the runtime in the filter when executing query? Currently property/field/methods are defined on DbContext because we know how to inject DbContext which is executing query to get actual value when running the query.

[AndersMalmgren]

No I am not, they are injected using IoC

service.AddTransient<IEntityAuthorization, AgreementAuthorization>();
service.AddTransient<IEntityAuthorization, OtherAuth>();
service.AddTransient<IEntityAuthorization, SomeAuth>();

[AndersMalmgren]

Why not have hooks that trigger before queries.

[smitpatel]

_entityAuthorization.ForEach(a => a.Build(builder));

This thing calls HasQueryFilter on Agreement entity multiple times. Last one wins!

[AndersMalmgren]

No it calls HasQueryFilter on whatever you define in IEntityAuthorization.Build

[AndersMalmgren]

HasQueryHook<TEntity>(Func<IQueryable<TEntity, IQueryable<TEntity>> queryHook);

Internally called when doing DbSet<TEntity> and filters dbset before its exposed to the user

[ajcvickers]

Notes from triage: the current design of query filters uses the DbContext as the anchor from which filter parameters are read. This works well in many cases, but has a couple of limitations:

• People don't get it. That is, many times we see code that attempts to hard-code the filter value into the model and then re-build the model for each context instance. This can be made to work, but only with terrible perf characteristics, which makes it a pit-of-failure.
• It requires a stronger coupling of the DbContext type to the entity types being used than is ideal--essentially the issue reported above.

Based on this, it might be worth designing and implementing query filters where the filter parameter is obtained in a different way--such as through a defined callback. This would be easier for many to understand and would not have a strong coupling to the context.

[AndersMalmgren]

We have already abstracted the Dbcontext away from the domain it uses a custom interface with a custom Set method so I might just write my own mechanic that hooks in our custom Set method. Only downside is that it will return a IQueryable instead of DbSet. But I can work around that by making sure everybody uses AddRange and similar on the abstracted dbcobtext rather than the DbSet

[joshcomley]

@AndersMalmgren this won't apply the query filters to any .Include(..) you use. Just want you to be aware of the security risk of doing that; your API may expose unwanted data via expands.

[AndersMalmgren]

No, If any entity has a relation to another entity that is under restriction that entity also needs to have filters. Thats ok in our case. Child and parents always have same restrictions.

We dont use Include but we project all queries to DTOs that might include children

[AndriySvyryd]

Also consider adding an overload to make the context parameter more obvious:

builder.HasQueryFilter<MyContext>((entity, context) => entity.TenantId == context.TenantId);