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

Support to DbCompiledModel and Multi tenancy #4410

Closed
ste4net opened this issue Jan 27, 2016 · 19 comments
Closed

Support to DbCompiledModel and Multi tenancy #4410

ste4net opened this issue Jan 27, 2016 · 19 comments

Comments

@ste4net
Copy link

ste4net commented Jan 27, 2016

Trying to port an MVC+EF5 web app to EF7 i realize that it is no more possible to instantiate a new Context passing a compiled model (DbComiledModel).
It was usefull (for us) for multi tenancy purpose.
This is an app with table dedicate to each tenant and tables (like asp identity tables) shared between all tenants.
Ex (codefirst mapping):

[...]
private static DbCompiledModel GetCompiled(DbConnection connection, string tenantSchema)
{
  builder.Entity<UserCompany>();
  builder.Entity<Invoice>().ToTable("Invoices", tenantSchema);
}
[...]
private static ConcurrentDictionary<string, DbCompiledModel> modelCache = new ConcurrentDictionary<string, DbCompiledModel>();
public static MyContext Create(string tenantSchema, DbConnection connection)
{
  DbCompiledModel compiledModel;
  if (tenantSchema == null || tenantSchema.Trim().Length == 0) tenantSchema = "/";
  compiledModel = modelCache.GetOrAdd(tenantSchema, (t) => GetCompiled(connection, t));
  return new MyContext(connection, compiledModel) { TenantSchema = tenantSchema };
}
[...]

Every time a new tenant is requested (depending by the user) Dbcontext is re-compiled for the new tenant and:
1-DbCompiledModel is saved in a ConcurrentDictionary to be used for next requests (for the same tenant)
2-DbContext saved in HttpRequest items to be used in from the same request.

This can be done using DbCompiledModel from EF5 because with this it is possible to cache the copiled model without using the EF5 caching system that can't be used due the presence of multi-tenancy/schema.

Maybe there is a better way do to this job in EF7 but i didn't found yet and i didn't found an alternative to DbCompiledModel.

@smitpatel
Copy link
Contributor

dupe of #1906 ?

@rowanmiller
Copy link
Contributor

You can do this already, here is what the code looks like

        private static IModel GetCompiled(DbConnection connection, string tenantSchema)
        {
            var builder = new ModelBuilder();
            builder.Entity<UserCompany>();
            builder.Entity<Invoice>().ToTable("Invoices", tenantSchema);
            return builder.Model;
        }
        private static ConcurrentDictionary<string, IModel> modelCache = new ConcurrentDictionary<string, IModel>();
        public static MyContext Create(string tenantSchema, DbConnection connection)
        {
            IModel compiledModel;
            if (tenantSchema == null || tenantSchema.Trim().Length == 0) tenantSchema = "/";
            compiledModel = modelCache.GetOrAdd(tenantSchema, (t) => GetCompiled(connection, t));

            var optionsBuilder = new DbContextOptionsBuilder<MyContext>();
            optionsBuilder.UseSqlServer(connection);
            optionsBuilder.UseModel(compiledModel);

            return new MyContext(optionsBuilder.Options) { TenantSchema = tenantSchema };
        }

You would not override OnConfiguring or OnModelCreating in the context, since all the config/model is done externally.

    public class MyContext : DbContext
    {
        public MyContext(DbContextOptions<MyContext> options)
            : base(options)
        { }

        ...
    }

@ste4net
Copy link
Author

ste4net commented Jan 27, 2016

Thank you very much.
So if I understood.
IModel is in fact the "compiled" model, all the heavy operations (mapping/binding) has been already done (at builder.Model call point) and results saved for next use.
If I am not wrong your updated code should be useful also for another type of multi-tenancy application; when we need different DB for each tenant.
Thank you very much.

@rowanmiller
Copy link
Contributor

@ste4net yes IModel is the same concept as DbCompiledModel. And yes, it works well for using the same model to connect to different databases too.

@rowanmiller
Copy link
Contributor

Closing this out as I think we have answered your question now.

@ste4net
Copy link
Author

ste4net commented Jan 29, 2016

Yes. You have answered. Thank you.
Now I have to understand what's the best way to intercept the Context creation with DI .
Looking the source code it's doesn't seems to be the right choice to use the default extension methods AddEntityFramework().AddDbContext because AddDbContext create a singleton instances of DbContextOptions and DbContextOptions Of T that is not what I need for this use case.
Maybe this should be enough to remove AddDbContext adding a scoped instance with custom activator.

       services.AddEntityFramework().AddSqlServer();
       services.AddScoped<MyContext>(MyCtxCreator.CreateForUserTenant);

@rowanmiller
Copy link
Contributor

@ste4net yes that is what you should do (it's essentially what AddDbContext does under the covers).

@unlimitedcoders
Copy link

I tried the above sample but it is not creating any tables when I am running context.Database.Migrate or EnsureCreated.

@rowanmiller
Copy link
Contributor

@unlimitedcoders can you open a new issue and include your complete code listing so that we can see what it going wrong (BTW Migrate wouldn't work unless you have migrations for the context in your project... but EnsureCreated should work)

@ste4net
Copy link
Author

ste4net commented Feb 17, 2016

This is just to complete the thread (hope it could be usefull for others).
Does not exist a parameterless constructor for ModelBuilder anyway i would like to use sql server conventions ( #3909 ) and i have to use the asp.net identity too.
So... this is the code i am writing sure it could be improved...

Note: I know there will be problem with Migrations but i create a specific post for that (#4583) .

 public class MyContextFactory : IDbContextFactory<ApplicationDbContext>
    {
        public ApplicationDbContext Create()
        {
        // THIS IS FOR MIGRATION ONLY
            string connection = GetConnectionStringHere();
            return CreateForTenant("Tenant_Code", connection);
        }
        public static ApplicationDbContext CreateForTenant(IServiceProvider srvProv)
        {
        //THIS IS THE METHOD CONFIGURED IN STARTUP
            ITenantProvider TntProv = srvProv.GetService<ITenantProvider>();
            string connection = GetConnectionStringHere();
            string currtenant = TntProv.GetCurrentTenant(); // <<<< GET TENANT CODE FROM SESSION/CACHE;
            return CreateForTenant(currtenant, connection);
        }
        private static ApplicationDbContext CreateForTenant(string TenantCode, string connection)
        {
            return ApplicationDbContext.Create(TenantCode, connection);
        }
    }
[...]

    public class ApplicationDbContext : IdentityDbContext<ApplicationUser, IdentityRole, string>
    {
        public string TenantSchema { get; set; }
        public ApplicationDbContext(DbContextOptions options) : base(options) { }
        private ApplicationDbContext() : base() { }
        private static IModel GetCompiled(string tenantSchema)
        {
            //https://github.com/aspnet/EntityFramework/issues/3909
            var serviceCollection = new ServiceCollection();
            serviceCollection.AddEntityFramework().AddSqlServer();
            var serviceProvider = serviceCollection.BuildServiceProvider();
            var coreConventionSetBuilder = new CoreConventionSetBuilder();
            var sqlConventionSetBuilder = new SqlServerConventionSetBuilder(new SqlServerTypeMapper());
            var conventionSet = sqlConventionSetBuilder.AddConventions(coreConventionSetBuilder.CreateConventionSet());

            var builder = new ModelBuilder(conventionSet);
            builder.Entity<UserCompany>();
        //Cal Manually OnModelCreating from base class to create model objects relatet di Asp.Net identity (not nice)
            (new ApplicationDbContext()).OnModelCreating(builder);  
            builder.Entity<Invoice>().ToTable("Invoices", tenantSchema);

            return builder.Model;
        }

        private static ConcurrentDictionary<string, IModel> modelCache = new ConcurrentDictionary<string, IModel>();

    private static ApplicationDbContext Create(string tenantSchema, string connectionstring)
        {

            IModel compiledModel;
            if (tenantSchema == null || tenantSchema.Trim().Length == 0) tenantSchema = "/";
            compiledModel = modelCache.GetOrAdd(tenantSchema, (t) => GetCompiled(t));

            var optionsBuilder = new DbContextOptionsBuilder<ApplicationDbContext>();

            optionsBuilder.UseSqlServer(connectionstring);
            optionsBuilder.UseModel(compiledModel);

            return new ApplicationDbContext(optionsBuilder.Options) { TenantSchema = tenantSchema };
        }

        protected override void OnModelCreating(ModelBuilder builder)
        {
            base.OnModelCreating(builder);
        }
    }

@sadjadbp
Copy link

This won't work any more with EF Core 2.1

@divega
Copy link
Contributor

divega commented May 12, 2018

@sadjadbp could clarify exactly what does not work and how it fails? Ideally please create a new issue including a repro.

@sadjadbp
Copy link

@divega cannot have one compiled model to connect to many DBs. Same issue that was discussed here, but the resolution won't work on 2.1 anymore. Actually this solution won't work on EF Core 2.0; I had to make a bit of modification to use SqlConventionsetBuilder.Build but that stopped working in 2.1.

@divega
Copy link
Contributor

divega commented May 12, 2018

@sadjadbp I may be missing something but the solution explained in this issue implements schema-per-tenant, with a single database and separate compiled model per tenant, which is kind of the opposite of what you are describing.

Which one do you want? If what you want to do is what is described in this issue, we can investigate here. Otherwise please do create a new issue. Also, how does it fail?

@sadjadbp
Copy link

What I'm looking for is same Compiled model different connection strings. I have a DB per tenant (elastic DB) but don't want to wait for model to be compiled per tenant request (or at least their request). What I ended up doing is inside OnConfiguring set the model that was created via previous call to OnModelBinding.

If you know an issue like this I'll link my question over there otherwise I'll create a new one (unless the answer is obvious)

@divega
Copy link
Contributor

divega commented May 12, 2018

@sadjadbp I believe if you just create the model normally in OnModelCreating and pass a different connection string to UseSqlServer for each tenant (either in OnConfiguring or passing a delegate that does this in AddDbContext) you should automatically get what you need.

Cc @ajcvickers in case he knows of any caveats with sharing the service container when the connection string changes.

@ajcvickers
Copy link
Contributor

@sadjadbp @divega The connection string does not influence the cache key, so it is normal and expected that multiple connections to different databases will share the same model, so long as they are using the same database provider. For a given provider, the only thing that influences the model cache by default is the DbContext type.

@ste4net
Copy link
Author

ste4net commented Jun 3, 2018

I was successfully using this approach with 2.0 .

Indeed there are issues now with 2.1 Calling base.OnModelCreating(builder);
there is an error with internal service provider.
I am studying the new 2.1 source code to understand what are services needed any suggestion will be really appreciated.

@ste4net
Copy link
Author

ste4net commented Jun 6, 2018

@divega I confirm that this approach can't work anymore starting from 2.1
During my tests i see that inheriting from IdentityDbContext you can't just use the OnModelCreating method to manually create the model outside the dbContext class.

It needs services and a serviceProvider not just a modelBuilder as parameter.

But… reading source code i see that EFCore already use exactly the same approach (ConcurrentDictionary) to cache the "compiled" model ( IModel ).
Searching a way to use the existing method i found that cache key is provided by IModelCacheKeyFactory that normally consider equals all IModel produced by the same DbContext you just have to make a custom key based on tenant.

This post from @bricelam saved me a lot of time.
https://stackoverflow.com/questions/41979215/how-to-implement-imodelcachekeyfactory-in-ef-core

Works perfectly with 2.1 too.

@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
None yet
Projects
None yet
Development

No branches or pull requests

7 participants