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

Q: Support for multi-tenant ? #123

Open
bobbyangers opened this issue Oct 29, 2021 · 24 comments
Open

Q: Support for multi-tenant ? #123

bobbyangers opened this issue Oct 29, 2021 · 24 comments
Labels
enhancement new feature or request up for grabs feature requires community contribution

Comments

@bobbyangers
Copy link

Would it be possible with the DB.DatabaseFor<entity>(dbName) to also have support for a "tenant".

This would enable the storage of the same entity type, but in different database depending on the request.

For example I would like to implement this and use MongoDB.Entities (and not MongoFramework)
Finbuckle.Multitenant

Maybe something like: DB.DatabaseFor<entity, ITenantInfo>(dbName)

or something implemented via the DbContext.

@dj-nitehawk
Copy link
Owner

as stated in the docs, multi-tenancy with db per tenant is currently not supported. and would require a significant investment to make it work and would have to break existing API. migrations are gonna be a troublesom also. and i've seen people mentioning there's a noticeable performance degradation when the number of databases in a single server is high. some sort of contention happening in a global level if i remember correctly.

however, i will investigate it when i get some time.

if you have any ideas, feel free to give it a go and submit a PR.

@dj-nitehawk dj-nitehawk added the enhancement new feature or request label Oct 29, 2021
@bobbyangers
Copy link
Author

Hi.

Of course the idea is not be to have "50" databases.

As a real case, we host a wepapi and want to have data isolation between the two or three branches

Looking at doc on Finbuckle, in the MongoFramework module, they have data isolation with one tenant per database and/or use a globalfilter to isolate tenant inside a database; in other words, the tenant info is prepend when fetching data. They also support hybrid, since each tenant have a connection string.

I'll fork the repo a make an attempt.

My initial thought would be to leverage the DatabaseFor idea. If an entity is decorated with a MultiTenant attribute, to include an extra "optional key" for indexing the connection string + a global filtering....

To be continued!

@dj-nitehawk
Copy link
Owner

dj-nitehawk commented Oct 29, 2021

we also have global filters: https://mongodb-entities.com/wiki/DB-Instances-Global-Filters.html
i'm having a look through the code now to see what can be done about db per tenant support.
but i think we need to specify the tenant id per request and divert the operations to the correct tenant's database prefixed with the tenant id.

@bobbyangers
Copy link
Author

I agree.

I think this can be achieved using a DbContext and resolve the correct db on a "per request" basis. Extracting the tenant info and creating a proper dbcontext, if using a repository.

More thoughts in progress.

@dj-nitehawk
Copy link
Owner

so... did a massive refactor to support db per tenant multi-tenancy support.
pushed an early preview to nuget if you'd like to help with testing. pretty sure i broke a couple of things. and couldn't figure out how to support multi-tenancy with one-to-many/many-to-many relationship stuff. at least not yet.

basically the usage will go like this:

public class Author : Entity
{
    public string Name { get; set; }
}

public class Book : Entity
{
    public string Title { get; set; }
}
//set which entity types should be stored in which dbs at app startup
//only needed when storing different entities in different databases/servers
//if all types are stored in a single database, this can be skipped
DB.DatabaseFor<Author>("AuthorData");
DB.DatabaseFor<Book>("BookData");

//configure a context for tenant A
//this should be done per request. 
//tenant name/prefix will come from the current request.
var ctxForTenantA = new TenantContext("TenantA");
ctxForTenantA.Init("AuthorData", "localhost"); //init calls can alternatively be done at startup if you have a list of tenant names/prefixes
ctxForTenantA.Init("BookData", "localhost");

//configure a context for tenant B
var ctxForTenantB = new TenantContext("TenantB");
ctxForTenantB.Init("AuthorData", "localhost");
ctxForTenantB.Init("BookData", "localhost");

//save entities for tenant A
var tenantABook1 = new Book { Title = "tenant a book 1" };
await ctxForTenantA.SaveAsync(tenantABook1);

var tenantAAuthor1 = new Author { Name = "tenant a author 1" };
await ctxForTenantA.SaveAsync(tenantAAuthor1);

//save entities for tenant B
var tenantBBook1 = new Book { Title = "tenant b book 1" };
await ctxForTenantB.SaveAsync(tenantBBook1);

var tenantBAuthor1 = new Author { Name = "tenant b author 1" };
await ctxForTenantB.SaveAsync(tenantBAuthor1);

@ahmednfwela
Copy link
Collaborator

why not reuse the same approach from ef core?
DBContext should include all info about database connection.
this means that static methods/mappings/properties are not valid.

so you can put tenancy info in the context, also that means that type to database mapping (DB. DatabaseFor) should be useless too.

as for relationships, ef core uses proxies and dynamic type generation to solve that

@bobbyangers
Copy link
Author

Hi @dj-nitehawk, going to be trying that shortly.

As for the relationships, maybe we can have a method on the

.AddChildrenAsync( parent, child/children ); //for one-many

.AddRelatedAsync( parent.child, child/children ); //for many-many

@dj-nitehawk
Copy link
Owner

dj-nitehawk commented Nov 1, 2021

@ahmednfwela well, true... but the static stuff is here to stay. cause i have around 10-12 projects using those and in no way i'm gonna touch those codebases anytime soon. so breaking the api is out of the question. also, you can sorta achieve a similar thing with a derived TenantContext which you can register with DI like so:

var builder = WebApplication.CreateBuilder();
builder.Services.AddScoped<MyDBContext>();

public class MyDBContext : TenantContext
{
    public MyDBContext(IConfiguration config, HttpContext httpCtx)
    {
        SetTenantPrefix(httpCtx.Request.Headers["tenant-id"]);
        Init("AuthorData", config["Databse:Authors:HostName"]);
        Init("BookData", config["Databse:Books:HostName"]);
    }
}

@bobbyangers i'll have another look at relationships soon. brain turned into mush trying to achieve multi-tenancy without breaking existing api ;-)

@ahmednfwela
Copy link
Collaborator

@dj-nitehawk there is no need to break existing api, it can simply wrap around a static singleton instance of the DBContext

@dj-nitehawk
Copy link
Owner

@ahmednfwela that is exactly what the TenantContext does. what issue do you see with the scoped tenant context approach i posted above? it's simple enough right?

@ahmednfwela
Copy link
Collaborator

ahmednfwela commented Nov 1, 2021

you can still see static fields and classes
e.g. https://github.com/dj-nitehawk/MongoDB.Entities/blob/master/MongoDB.Entities/DB/DB.cs#L37
and

private static readonly ConcurrentDictionary<Type, IMongoDatabase> TypeToDBMap = new();

this type to DB map should be done per DBContext, not statically.
or even better, entirely removed

@dj-nitehawk
Copy link
Owner

dj-nitehawk commented Nov 1, 2021

yes took a different approach in the multi-tenancy branch

@ahmednfwela
Copy link
Collaborator

yes, but the fact that we are storing anything at all in the static DB class
https://github.com/dj-nitehawk/MongoDB.Entities/blob/multi-tenancy/MongoDB.Entities/DB/DB.cs#L36
is anti-pattern and is the source of all evil for all this

@ahmednfwela
Copy link
Collaborator

The current DBContext api relies on the static DB class, but it should be the other way around

@dj-nitehawk
Copy link
Owner

dj-nitehawk commented Nov 1, 2021

maybe around v25 we could make it DBContext only and get rid of the static methods. but the static methods are just sooo convenient to use. let's see... if i get a full week of time off from my regular job, i'll look into a full revamp. until then, no breaking changes ;-)
you're more than welcome to have a crack at it in the meantime...

@ahmednfwela
Copy link
Collaborator

Great ! I just wanted to see if you agree to the changes in general.
I will try rebuilding the API to also somewhat match how EFCore does it.
Thanks for your great work !

@dj-nitehawk
Copy link
Owner

awesome! hopefully there won't be too many breaking changes to existing consumers...
good luck!!!

@bobbyangers
Copy link
Author

As @ahmednfwela was suggesting....

If the "static" implementation refer to a DBContext that would be instanciated in the INIT, and move all the transaction aware and db connection management inside the DBContext, I am sure we can expect to find a way to keep the existing API as is.

The Automapper did a similar move, that is moving everything from static implementation to (ie singleton) to transient injectable objects. And it solved quite a bit of headaches.

With multi-threading and high volume apps, it is safer to "Create-Use-Dispose" objects.

Keeping the existing static is important, but I am sure that following the current trend, Create-Use-Dispose, it will keep your great library around longer and will encourage it's usage.

@dj-nitehawk
Copy link
Owner

yeah, i'm leaning towards that too... just need to find the time to do a rewrite...
hopefully @ahmednfwela has something cooking too.
let's see...

@bobbyangers
Copy link
Author

bobbyangers commented Nov 4, 2021

Here is an example of static being not so great....

DB.InitAsync(_dbName, settings) behing called at the startup.

Later

        var settings = MongoClientSettings.FromConnectionString("mongodb://localhost:27017");
        settings.ClusterConfigurator = cb => {
              .Subscribe<CommandStartedEvent>(e =>
                 {
                      logger.WriteLine($"{e.GetType().Name} {e.CommandName} - {e.Command.ToJson()}");
                 });
        };

        var client = new MongoClient(settings);

        var db = client.GetDatabase(_dbName);

        await db.CreateCollectionAsync("DemoNative");

        var sut = db.GetCollection<DemoNative>("DemoNative");

        var data = _fixture.Create<DemoNative>();
        await sut.InsertOneAsync(data);
        ShowResult("this is a test");
    }

Then the susbscribe is not considered because the internal

       internal static async Task Initialize(...)
       {
            // ...
            if (DB.dbs.ContainsKey(dbName))
              return;
       }

And since the logger is diposable..... then we get ObjectDisposedException

Is there a way to expose the skipNetworkPing and for a replace in the DB.dbs ? Maybe add it as an optional parameter to InitAsync ?

Or a way to manually clear/remove the entry ?

For the moment I used this in conjuction with Dependency Injection.... not great... but it works if I need the "logging" to work.

public static class DbContextExtensions
{

    public static void ClearDbs(string name)
    {
        var target = typeof(DB);

        var f = target.GetField("dbs", BindingFlags.GetField | BindingFlags.NonPublic | BindingFlags.Static);

        var dbs = (ConcurrentDictionary<string, IMongoDatabase>)f.GetValue(null);
        dbs.TryRemove(name, out IMongoDatabase _);
    }
}

@dj-nitehawk
Copy link
Owner

hopefully #125 will sort out those headaches...
looking good buddy @ahmednfwela 😍👌

@dj-nitehawk dj-nitehawk added the wip work in progress label Nov 17, 2021
@rainxh11
Copy link

That's my solution for now, multitenancy using the same DB, Autofac Multinancy

Custom DBContext:

    public class ClientDbContext : DBContext
    {
        public string TenantId { get; }

        public ClientDbContext(string tenantId)
        {
            TenantId = tenantId;
            SetGlobalFilterForBaseClass<ClientEntity>(
                filter: b => b.TenantID == tenantId,
                prepend: true);

            SetGlobalFilterForBaseClass<ClientFile>(
                filter: b => b.TenantID == tenantId,
                prepend: true);
        }

        protected override Action<T> OnBeforeSave<T>()
        {
            Action<ClientEntity> action = f => { f.TenantID = TenantId; };
            return action as Action<T>;
        }

        protected override Action<UpdateBase<T>> OnBeforeUpdate<T>()
        {
            Action<UpdateBase<ClientEntity>> action = update =>
            {
                update.AddModification(f => f.TenantID, TenantId);
            };
            return action as Action<UpdateBase<T>>;
        }
    }

Tenant Identification Strategy:

    public class SchoolTenantIdentificationStrategy : ITenantIdentificationStrategy
    {
        private IHttpContextAccessor _accessor;

        public SchoolTenantIdentificationStrategy(IHttpContextAccessor accessor)
        {
            _accessor = accessor;
        }

        public bool TryIdentifyTenant(out object tenantId)
        {
            var clientId = _accessor.HttpContext?.User.Claims.First(x => x.Type == "TenantId").Value;
            tenantId = clientId;

            if (clientId is null)
                return false;
            return true;
        }
    }

So far the performance is great

@mkgn
Copy link

mkgn commented Dec 7, 2023

As per the documentation at https://mongodb-entities.com/wiki/DB-Instances.html, I am trying to create DBContext per request based on TenantId (identified via a Middleware). The DBContext is returned via below method per request

    public MongoDbConfigurationBuilder AddMongoDbMultiTenantContext()
    {
        services.TryAddScoped(provider =>
        {
            var tenantService = provider.GetRequiredService<ITenantService>();
            var settings = MongoClientSettings.FromConnectionString(tenantService.CurrentTenant.ConnectionString);
            return new MongoDbContext(tenantService.CurrentTenant.Database,settings,new MongoDB.Entities.ModifiedBy
            {
                UserID="1234567",
                UserName="username"
            });
        });

        return this;
}

I inherited DBContext and created MongoDbContext since I need to add more features to it in future.
When debugging I see the MongoDbContext gets passed different connectionstrings but all data is getting saved in the same database.
This is because internally DBContext simply use DB... which has got initialized to whatever the first connected database.

Seems like I can't use what's mentioned in https://mongodb-entities.com/wiki/Multiple-Databases.html for my purpose.

Am I correct to assume that the library cannot switch databases per request as above?

@dj-nitehawk
Copy link
Owner

@mkgn yeah db per tenant is not supported with the current state of the library.
and can't be added easily without a complete rewrite and breaking existing user code.
don't have the time nor the motivation to do a rewrite anytime soon :-(

PRs are welcome from the community if it can be done in a way that's not too difficult to migrate existing code bases.

@dj-nitehawk dj-nitehawk added up for grabs feature requires community contribution and removed wip work in progress labels Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement new feature or request up for grabs feature requires community contribution
Projects
None yet
Development

No branches or pull requests

5 participants