Finbuckle / Finbuckle.MultiTenant

Finbuckle.MultiTenant is an open-source multitenancy middleware library for .NET. It enables tenant resolution, per-tenant app behavior, and per-tenant data isolation.
https://www.finbuckle.com/multitenant
Apache License 2.0
1.31k stars 267 forks source link

Composite primary key errors #835

Open shainegordon opened 4 months ago

shainegordon commented 4 months ago

"Reopening" this as this does seem to be an issue, or it's a misunderstanding.

Using a standard reference implementation of composite keys, you'll get an error when calling Add on an Entity in the DBContext:

InvalidOperationException: Unable to track an entity of type 'Client' because its primary key property 'TenantId' is null.

I have created a PR/fork of the sample project that can reproduce this issue: https://github.com/Finbuckle/Finbuckle.MultiTenant/pull/834

AndrewTriesToCode commented 3 months ago

Thanks for creating the fork. Just out of curiosity have you tried different database providers at all? I'm trying to figure out the scope of the issue and if it is specific to one provider or something similar. It may be more fundamental than that.

Also do you think this is something that could impact a non Finbuckle.MultiTenant db context? Is there a way to arbitrarily reproduce it by setting a primary key as a composite on regular db context?

shainegordon commented 3 months ago

Thanks for creating the fork. Just out of curiosity have you tried different database providers at all? I'm trying to figure out the scope of the issue and if it is specific to one provider or something similar. It may be more fundamental than that.

Also do you think this is something that could impact a non Finbuckle.MultiTenant db context? Is there a way to arbitrarily reproduce it by setting a primary key as a composite on regular db context?

So I initially encountered this using the Npgsql.EntityFrameworkCore.PostgreSQL provider.

I've never personally created a "virtual" composite key before.

I'm also trying to decide how to proceed on my side. i.e is it fine for the PK/FK relationship to just be on the ID field, and just use .AdjustUniqueIndexes().AdjustIndexes()

gitruhul commented 3 months ago

I tried to add tenantId as a key in Postgres and SQLSERVER. The issue exists for both the DBs

System.InvalidOperationException: Unable to track an entity of type 'Product' because its primary key property 'TenantId' is null.
AndrewTriesToCode commented 3 months ago

It’s an issue at the tracker level for any db provider.

There is a workaround example from pr #834 The idea is you attached in the detached state, set the tenant id shadow property, then change the state to added.

app.MapGet("/create-client", (ApplicationDbContext applicationDbContext, HttpContext httpContext) =>
{
    var client = new Client { Name = "Client 1" };
    var tenantId = httpContext.GetMultiTenantContext<AppTenantInfo>().TenantInfo?.Id ?? "null";
    applicationDbContext.Entry(client).Property("TenantId").CurrentValue = tenantId;
    applicationDbContext.Add(Client);

   // set here or somewhere else if desired...
   applicationDbContext.TenantMismatchMode = TenantMismatchMode.Overwrite;

   applicationDbContext.SaveChanges();

    return Results.Ok("Client created");
});
gitruhul commented 3 months ago

So, Do I need to add this work around in every controller and each API?

shainegordon commented 3 months ago

So, Do I need to add this work around in every controller and each API?

basically yes, but you can get EFCore to fix/automated this for you, doing something like this

In your application context

       public override async Task<int> SaveChangesAsync(bool acceptAllChangesOnSuccess, CancellationToken cancellationToken = default)
        {
            ReplaceTenantIdPlaceholders();
            return await base.SaveChangesAsync(acceptAllChangesOnSuccess, cancellationToken);
        }

        private void ReplaceTenantIdPlaceholders()
        {
            var addedMultiTenantEntities = ChangeTracker.Entries().
                Where(e => e.State == EntityState.Added).
                Where(e => e.Metadata.IsMultiTenant()).ToList();

            // ensure tenant context is valid
            if (addedMultiTenantEntities.Count > 0 && TenantInfo == null)
            {
                throw new MultiTenantException("MultiTenant Entity cannot be changed if TenantInfo is null.");
            }

            var mismatchedAdded = addedMultiTenantEntities.
                Where(e => (string?)e.Property("TenantId").CurrentValue == MultiTenantEntity.TenantIdPlaceholder &&
                           (string?)e.Property("TenantId").CurrentValue != TenantInfo!.Id).ToList();

            foreach (var e in mismatchedAdded)
            {
                e.Property("TenantId").CurrentValue = TenantInfo!.Id;
            }
        }

In your entity

    public const string TenantIdPlaceholder = "THIS_VALUE_GETS_REPLACED_BY_FINBUCKLE";

    // ReSharper disable once UnusedMember.Global
    public string TenantId { get; set; } = TenantIdPlaceholder;