dj-nitehawk / MongoDB.Entities

A data access library for MongoDB with an elegant api, LINQ support and built-in entity relationship management
https://mongodb-entities.com
MIT License
536 stars 69 forks source link

Q: Support for multi-tenant ? #123

Open bobbyangers opened 2 years ago

bobbyangers commented 2 years ago

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 commented 2 years ago

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.

bobbyangers commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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

dj-nitehawk commented 2 years ago

@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 commented 2 years ago

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 https://github.com/dj-nitehawk/MongoDB.Entities/blob/322563f6d943f20b0263b14fdaddcb77dd31b1da/MongoDB.Entities/Core/TypeMap.cs#L9

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

dj-nitehawk commented 2 years ago

yes took a different approach in the multi-tenancy branch

ahmednfwela commented 2 years ago

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 commented 2 years ago

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

dj-nitehawk commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

bobbyangers commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

rainxh11 commented 2 years ago

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 commented 7 months ago

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 commented 7 months ago

@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.