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

Query filter samples don’t make sense #12375

Closed
granthoff1107 opened this issue Jun 15, 2018 · 9 comments
Closed

Query filter samples don’t make sense #12375

granthoff1107 opened this issue Jun 15, 2018 · 9 comments
Labels
closed-no-further-action The issue is closed and no further action is planned. customer-reported

Comments

@granthoff1107
Copy link

granthoff1107 commented Jun 15, 2018

The tenantId queryfilter samples make no sense

       #region Configuration
        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.Entity<Blog>().Property<string>("TenantId").HasField("_tenantId");

            // Configure entity filters
            modelBuilder.Entity<Blog>().HasQueryFilter(b => EF.Property<string>(b, "TenantId") == _tenantId);
            modelBuilder.Entity<Post>().HasQueryFilter(p => !p.IsDeleted);
        }
        #endregion

The tenantId filter checks to see if the property which backed by _tenantId is equivalent to _tenantId, this should always return true

@ajcvickers
Copy link
Member

@granthoff1107 TenantId is property on the entity type--in this base Blog. While it is backed by a _tenantId field, this is not the same _tenantId field that is used in the filter; the one in the filter is a member of the DbContext class. This one is captured into the query expression such that it can be changed on each context instance resulting the context filtering queries based on whatever _tenantId is set on the current instance.

@ajcvickers ajcvickers added the closed-no-further-action The issue is closed and no further action is planned. label Jun 15, 2018
@granthoff1107
Copy link
Author

How so if you look at the blog class the tenantId is private, it doesn’t look like it’s set anywhere this is unclear from the documentation

@smitpatel
Copy link
Member

modelBuilder.Entity().Property("TenantId").HasField("_tenantId");

This line creates a shadow property named TenantId (which is being used in query filter), and assign the private field _tenantId to store its data. This way, while TenantId filter exists in data layer, to the consumers of the Blog class there is no exposure.

@granthoff1107
Copy link
Author

@smitpatel, okay so the tenantId is assigned per say by some authorization service but who makes the assignment to the tenantId in the blog class?

@smitpatel
Copy link
Member

smitpatel commented Jun 15, 2018

@granthoff1107 - That is something user has to do. Generally it is done by overriding SaveChanges in derived DbContext.

Like showed in sample
https://github.com/aspnet/EntityFrameworkCore/blob/444a089a26bb3b31e783ce1b78ab7024652acdb1/samples/QueryFilters/Program.cs#L153-L170

We have tracking issue #10862 making it more first class.

@granthoff1107
Copy link
Author

The sample you provided sets the property to equal to the same property used in the DbContext, I still don’t see how this would evaluate to anything but true?

@granthoff1107
Copy link
Author

Maybe I’m confused what tenancy is used for. I thought it was primarily used to filter values by companies, users etc... I don’t understand why would someone one override savechanges to set a field on a value which is only backed by a property?

@mguinness
Copy link

mguinness commented Jun 26, 2018

@granthoff1107 Yep the example is confusing all right. The use case you suggested is covered in Include the IsDeleted values when user is admin where the field is defined in the DbContext, not the model.

@vanillajonathan
Copy link
Contributor

The comparison of the property is against _tenantId which is an field variable of the DbContext class not the entity, hence it will not always evaluate to true.

Here is a fuller example that illustrates it better:

    public class BlogContext : DbContext
    {
        private string _tenantId; // This field is what the entity is compared against

        public BlogContext(DbContextOptions options)
            : base(options)
        {
            _tenantId = "1"; // Hard-coded in this example case
        }

        public DbSet<Blog> Blogs { get; set; }

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.Entity<Blog>().Property<string>("TenantId").HasField("_tenantId");

            modelBuilder.Entity<Blog>().HasQueryFilter(b => EF.Property<string>(b, "TenantId") == _tenantId);
        }
    }

The _tenantId field can be set in the constructor. Perhaps by dependency injection of perhaps IHttpContextAccessor.
Maybe something like this:

        public BlogContext(DbContextOptions options, IHttpContextAccessor httpContextAccessor)
            : base(options)
        {
            _tenantId = httpContextAccessor.HttpContext.User.FindFirstValue("TenantId");
        }

@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
Labels
closed-no-further-action The issue is closed and no further action is planned. customer-reported
Projects
None yet
Development

No branches or pull requests

5 participants