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

Automatically perform required conversions when query filter is defined on interface or base type #10257

Closed
nphmuller opened this issue Nov 10, 2017 · 5 comments

Comments

@nphmuller
Copy link

Say I have the following interface that can be implemented on any entity that I want to support soft-deletion.

public interface ISoftDeletableEntity
{
    bool IsDeleted { get; set; }
}

It would be nice to be able to define a global query filter for any entity type that implements this interface like this:

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
    modelBuilder.Entity<ISoftDeletableEntity>().HasQueryFilter(e => e.IsDeleted == false);
}

Currently this throws the following exception: System.ArgumentException: 'The entity type 'MyApp.ISoftDeletableEntity' provided for the argument 'clrType' must be a reference type.'

@ajcvickers
Copy link
Member

@nphmuller This is something that can be done with bulk configuration on the mapped entity types:

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
    foreach (var entityType in modelBuilder.Model.GetEntityTypes()
        .Where(e => typeof(ISoftDeletableEntity).IsAssignableFrom(e.ClrType)))
    {
        modelBuilder
            .Entity(entityType.ClrType)
            .HasQueryFilter(ConvertFilterExpression<ISoftDeletableEntity>(e => !e.IsDeleted, entityType.ClrType));
    }
}

private static LambdaExpression ConvertFilterExpression<TInterface>(
    Expression<Func<TInterface, bool>> filterExpression, 
    Type entityType)
{
    var newParam = Expression.Parameter(entityType);
    var newBody = ReplacingExpressionVisitor.Replace(filterExpression.Parameters.Single(), newParam, filterExpression.Body);

    return Expression.Lambda(newBody, newParam);
}

Making a better experience around this is covered by #9117 and #6787.

The code is more complicated in this case than in many others because it requires constructing a lambda expression where the parameter is validated to be the exact entity type--not the interface. I'm going to re-purpose this bug to automatically handle base types/interfaces in EF code. After this issue is fixed it should be possible to simplify the above to:

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
    Expression<Func<ISoftDeletableEntity, bool>> expression = e => !e.IsDeleted;

    foreach (var entityType in modelBuilder.Model.GetEntityTypes()
        .Where(e => typeof(ISoftDeletableEntity).IsAssignableFrom(e.ClrType)))
    {
        modelBuilder.Entity(entityType.ClrType).HasQueryFilter(expression);
    }
}

@ajcvickers ajcvickers changed the title Allow model-level filters by base-type or interface. Automatically perform required conversions when filter is defined on interface of base type Nov 10, 2017
@ajcvickers ajcvickers changed the title Automatically perform required conversions when filter is defined on interface of base type Automatically perform required conversions when query filter is defined on interface or base type Nov 10, 2017
@HappyNomad
Copy link

@ajcvickers Wouldn't be more efficient to set IMutableEntityType.QueryFilter like my example at #8881 (comment)? That way you don't look up every affected entity type twice. Please "Automatically perform required conversions when query filter is defined" in that manner, too.

@nphmuller
Copy link
Author

nphmuller commented Nov 12, 2017

@ajcvickers
Nice. Was hoping it was possible already, but got stuck rewriting the expression. Thanks for the sample. It helps out a lot!

Totally agree with the repurpose. The sample after the fix is way easier to write.

@nphmuller
Copy link
Author

nphmuller commented Nov 13, 2017

I've set up a Gist which shows my query filter use-case and the code I had to write to get it working.
https://gist.github.com/nphmuller/8891c315d79aaaf720f9164cd0f10400
https://gist.github.com/nphmuller/05ff66dfa67e1d02cdefcd785661a34d

@ajcvickers
Copy link
Member

Note from triage: if we implement this, then it will be as part of general mapping configuration for all uses of an interface, tracked by #6787.

@ajcvickers ajcvickers removed this from the Backlog milestone Oct 27, 2021
@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants