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

Composite primary key errors #835

Open
shainegordon opened this issue May 28, 2024 · 6 comments
Open

Composite primary key errors #835

shainegordon opened this issue May 28, 2024 · 6 comments
Labels

Comments

@shainegordon
Copy link

"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: #834

@AndrewTriesToCode
Copy link
Sponsor Contributor

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
Copy link
Author

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
Copy link

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
Copy link
Sponsor Contributor

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
Copy link

gitruhul commented Jul 2, 2024

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

@shainegordon
Copy link
Author

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;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

3 participants