dotnet / EntityFramework.Docs

Documentation for Entity Framework Core and Entity Framework 6
https://docs.microsoft.com/ef/
Creative Commons Attribution 4.0 International
1.63k stars 1.96k forks source link

Clarify using query filters for multi-tenancy #1361

Open YvonneArnoldus opened 5 years ago

YvonneArnoldus commented 5 years ago

I tried this example in EF Core 2.2.2 but it does not seem to work.

I see my _tenantId get changed in MyDbContext but when i do a _context.MySet..AsQueryable()..Skip(pageSize).Take(pageCount).ToListAsync(). The result I get are not for the _tenantId but for the tenant Id did with another browser.


Document Details

Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

asztal commented 5 years ago

https://docs.microsoft.com/en-us/dotnet/api/microsoft.entityframeworkcore.dbcontext.onmodelcreating?view=efcore-2.1

The resulting model may be cached and re-used for subsequent instances of your derived context.

This very much suggests that using a query filter for dynamically changing values like a tenant ID (which might be different per DbContext instance) will probably not work... Maybe a contributor can correct me.

AndrewTriesToCode commented 5 years ago

This can work if you define a new Model Cache Key Factory (https://docs.microsoft.com/en-us/ef/core/modeling/dynamic-model) that provides a unique key per tenant. Alternatively you can set up the global query to accept a lambda parameter rather than a constant tenant expression so that the same model is used but this is harder to work with.

sevenmay commented 5 years ago

So do these two documents have conflicting information?

  1. https://docs.microsoft.com/en-us/ef/core/querying/filters
  2. https://docs.microsoft.com/en-us/ef/core/modeling/dynamic-model

Isn't the example in (1) sufficient to implement a multi-tenant scenario? Do we need to implement a dynamic IModelCacheKeyFactory?

In an asp.net core app where the DbContext is created for every request how do the model caching works?

To be clear it works fine for me in 3.0 preview 5.

AndrewTriesToCode commented 5 years ago

In my opinion they don't conflict, but the multi tenant example in the query filter document leave out some important detail. From my observations in 2.X what happens is the first use of a dbcontext will create the model and cache it. If the model has something like this from the example:

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

The _tenantID at that moment will be frozen into the model and used for future dbcontexts (even if it is a different tenant). So modifying the cache key factory can solve this by having a different model created and cached per tenant... which might not work well if you have a large number of tenants.

Maybe EFCore 3 improved on that?

Another idea I use in my library (which someone else suggested... I can't take credit for this) is to have a single model which uses a lambda instead of a constant. Something like this:

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

I don't know if that line will work as-is. I build the expression tree manually due to limitations in the implicit lambda -> expression conversion and it works for me.

bhalbright commented 5 years ago

Just an opinion but it seems this document might be better served to just show a simpler example than multi-tenancy and leave that documentation for another place. I understand global query filters after reading this but I was confused about the multi-tenancy with DbContext since there's only a little information here on the page.

davisnw commented 4 years ago

It seems like a rather critical issue that the tenantId be properly used and it's value not statically captured in the lambda expression. Documentation and github sample need to be updated if the approach shown in both places is not reliable.

Costo commented 4 years ago

The doc states: "Note the use of a DbContext instance level field: _tenantId used to set the current tenant. Model-level filters will use the value from the correct context instance (that is, the instance that is executing the query)."

This seems to contradict the issue discussed here. I'm playing with this to implement multi-tenancy and it seems to work fine as-is.

hexdump2002 commented 4 years ago

The doc states: "Note the use of a DbContext instance level field: _tenantId used to set the current tenant. Model-level filters will use the value from the correct context instance (that is, the instance that is executing the query)."

This seems to contradict the issue discussed here. I'm playing with this to implement multi-tenancy and it seems to work fine as-is.

Could you explain how do you inject the tenantId into the DbContext instance? In my case I'm creating a Web API and need to get it from the request body.

mhosman commented 4 years ago

Hey @hexdump2002, watch this video: https://www.youtube.com/watch?v=-1cTReZ9lzY

taylorchasewhite commented 2 years ago

Hey @hexdump2002, watch this video: https://www.youtube.com/watch?v=-1cTReZ9lzY

This is an hour long video, can you be more specific to @hexdump2002's question? I just watched about half of it and have not seen the answer to his question.