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

Improve usability of global query filters #21459

Open
11 tasks
ajcvickers opened this issue Jun 29, 2020 · 8 comments
Open
11 tasks

Improve usability of global query filters #21459

ajcvickers opened this issue Jun 29, 2020 · 8 comments
Labels
area-query composite-issue A grouping of multiple related issues into one issue needs-design type-enhancement
Milestone

Comments

@ajcvickers
Copy link
Member

ajcvickers commented Jun 29, 2020

This is a grouping of related issues. Feel free to vote (👍) for this issue to indicate that this is an area that you think we should spend time on, but consider also voting for individual issues for things you consider especially important.


There are several areas in which the experience of query filters could be improved. This issue collects query filter usability into a single place with reference to specific issues.

In a nutshell, it should be possible to specify multiple different query filters, enable and disable them independently, and be able to parameterize each filter in an obvious way.

Backlog

@mguinness
Copy link

mguinness commented Aug 8, 2020

I'll add a use case since I think it might be common, but I'm unsure which approach is best suited. It can be beneficial to control filters by role/policy, i.e. if you have an archive flag on an entity then for users it would be filtered but for admins it would be ignored.

@jack775544
Copy link

To add my 2 cents to this discussion I do really like the suggestions that were proposed in #23067. More often then not I find myself not wanting global filters by default but instead being able to add optional global filters as required.

Imagine having a posts table with the following contents

public class Post
{
	string Contents { get; set; }
	bool IsSoftDeleted { get; set; }
	Guid AuthorId { get; set; }
}

Something I often find I want is to dynamically add filters based on the requirements that I have at the time. So if I had a rule that only post authors could see their own posts and users can't see soft deleted posts then I would like to do something like this.

void AddSoftDeleteFilter(MyDbContext ctx)
{
	ctx.Post.AddFilter(x => !x.IsSoftDeleted);
}

void AddAuthorFilter(MyDbContext ctx, Guid currentUserId)
{
	ctx.Post.AddFilter(x => currentUserId == x.AuthorId);
}

AddSoftDeleteFilter(ctx);
AddAuthorFilter(ctx, currentUser.Id);

ctx.Post.ToList();
ctx.SomeOtherEntity.Include(x => x.Posts).ToList(); // Would also have filters applied

This sort of pattern would allow for much more fine grained use of filters rather than hammer of the current model based approach that is in use right now.


Also as a side note the requirement that filters can't be used on inherited entities is another pain point for me. I often end up inheriting from AspNetUsers for my user classes because of this filters are unusable on them.

@baratgabor
Copy link

I think, with respect to query filter ignoring, #17347 would still leave an arguably important use case unsolved. That is, when we want to ignore specific query filters in a manner that is transparent to the consumers of the DbContext.

These filters are often used to move filtering presets from repositories into the DbContext, e.g. the often cited soft-deletion. In this scenario the expectation is that the DbSet exposed by DbContext is fully set up for queries on a given entity, without needing any further abstractions or customizations.

Example

Lets say I want to implement soft-deletion inside my DbContext, and I have a Partner entity that can be soft-deleted. These Partner entities are referenced via a required navigation property in Order entities. Now, if a Partner is soft-deleted (to hide it in the partner listing, prevent further orders to be created with the partner, etc.), most likely I would still want to show the already created Orders that happen to reference the soft-deleted partner, as a cross-cutting concern with respect to order querying.

But if the improved query filter ignoring still only uses the IgnoreQueryFilter() method that returns an IQueryable, I can't implement this cross-cutting concern right into the exposed DbSets, defeating the purpose of moving such filtering logic into the DbContext (since if I still need an abstraction layer, I might as well just use the old method of adding filters to IQueryable base queries inside my repositories).

My two cents is that the global query filters feature should focus on more global concerns like these, instead of per-query concerns on IQueryables.

mnijholt added a commit to mnijholt/efcore that referenced this issue May 13, 2021
- Configure multiple query filters on a entity referenced by name.
- Ignore individual filters by name.

The current implmentation of query filter is kept but when ignoring the filters using the current extension method will ignore all filters (so also the named).

Fixes dotnet#8576 dotnet#10275 dotnet#21459
@AndriySvyryd AndriySvyryd modified the milestones: Backlog, Epics Jan 14, 2022
@pdevito3
Copy link

I ran into issues with this on a recent project as well. Query filters are not intuitive at all as is IMHO.

One issue we didn't realize before coming across it in the docs after the fact is that you can't define multiple query filters on the same entity. For instance, I'd expect the below to both filter deleted entities and tenants:

public class RecipesDbContext : DbContext
{
    //...

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        base.OnModelCreating(modelBuilder);
         modelBuilder.FilterSoftDeletedRecords();
         modelBuilder.FilterByTenant();
    }
}

public static class Extensions
{
    public static void FilterSoftDeletedRecords(this ModelBuilder modelBuilder)
    {
        Expression<Func<BaseEntity, bool>> filterExpr = e => !e.IsDeleted;
        foreach (var mutableEntityType in modelBuilder.Model.GetEntityTypes()
            .Where(m => m.ClrType.IsAssignableTo(typeof(BaseEntity))))
        {
            // modify expression to handle correct child type
            var parameter = Expression.Parameter(mutableEntityType.ClrType);
            var body = ReplacingExpressionVisitor
                .Replace(filterExpr.Parameters.First(), parameter, filterExpr.Body);
            var lambdaExpression = Expression.Lambda(body, parameter);

            // set filter
            mutableEntityType.SetQueryFilter(lambdaExpression);
        }
    }

    // also tenant filter here
}

Additionally, we ran into a similar issue as this. Where we need a private field to get it keep the proper scope when filtering by tenant.

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);
    }
}

Would be really great to have some enhancements here in EF Core 7

@pantonis
Copy link

Does the 24445 implemented in EF Core 7.xxx

@ajcvickers
Copy link
Member Author

@pantonis No. That's why the issue is still open and in the Backlog milestone.

@selcukkutuk
Copy link

Are the jobs on this backlog list not important enough or not in enough demand? When the global query filter was first announced, I thought it was not functional enough and was announced a little hastily. Looking at the backlog list today, I seem to see that I was right. Is there a planned road map for the development of this feature, which is great in terms of idea?

@roji
Copy link
Member

roji commented Sep 8, 2023

@selcukkutuk the number of issues on our backlog - for global query filters and in general - isn't an indication of how mature anything is; EF is a very widely-used project, and we get very large numbers of issues, many of which end up in the backlog. Many people are using global query filters successfully, and although this feature could be improved, we have not seen huge numbers of user votes/interest on the related issues that would justify prioritizing them above other work.

I'd suggest concentrating on the specific issues which you need, and making sure you vote on them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-query composite-issue A grouping of multiple related issues into one issue needs-design type-enhancement
Projects
None yet
Development

No branches or pull requests

9 participants