dotnet / efcore

EF Core is a modern object-database mapper for .NET. It supports LINQ queries, change tracking, updates, and schema migrations.
https://docs.microsoft.com/ef/
MIT License
13.68k stars 3.16k forks source link

Saving entities with composite primary key including a foreign key is not working #6321

Closed audacity76 closed 1 year ago

audacity76 commented 8 years ago

Steps to reproduce

I have a multi tenant database. Because of query performance reasons I added the tenant's Id to each primary key so all primary keys consist of 2 properties: Id + TenantId. The TenantId gets set by the database (via session context and default value). The TenantId is also a foreign key to the Tenant entity.

The issue

Saving any entity is impossible because the TenantId gets set during save to a wrong TenantId.

Problem located inside the ValueGenerationManager.cs:

private IEnumerable<Tuple<IProperty, bool>> FindProperties(InternalEntityEntry entry)
{
            foreach (var property in entry.EntityType.GetProperties())
            {
                var isForeignKey = property.IsForeignKey();

                if ((property.RequiresValueGenerator || isForeignKey) // <----------------- HERE
                    && property.ClrType.IsDefaultValue(entry[property]))
                {
                    yield return Tuple.Create(property, isForeignKey);
                }
            }
}

Further technical details

EF Core version: 1.0.0 Operating system: Win10 Visual Studio version: 2015

ajcvickers commented 8 years ago

EF Team Triage: This issue is lacking enough information for us to be able to effectively triage it. In particular, it is missing the following information requested in the new issue template. Can you please provide this information?

Steps to reproduce

Ideally include a complete code listing that we can run to reproduce the issue. Alternatively, you can provide a project/solution that we can run.

BTW we're not just doing this to be mean :smile:... we get a lot traffic on this project and it takes time to attempt to reproduce an issue based on fragments of information. In addition, our attempt is often unsuccessful as the exact conditions required to hit the issue are often not explicitly included in the code provided. To ensure we maximize the time we have to work on fixing bugs, implementing new features, etc. we ask that folks give us a self contained way to reproduce an issue.

For a guide on submitting good bug reports, read Painless Bug Tracking.

audacity76 commented 8 years ago

Ok. Here we go...

public class Program
{
    public static void Main(string[] args)
    {
        using (var db = new BloggingContext())
        {
            if (!db.Tenants.Any())
            {
                db.Tenants.Add(new Tenant { Name = "Microsoft" });
                db.SaveChanges();
                Console.WriteLine($"Tenant saved");
            }

            db.Blogs.Add(new Blog { Url = "http://blogs.msdn.com/adonet" });
            db.SaveChanges(); // <-------- Doesn't work
            Console.WriteLine($"Blog saved");

            Console.WriteLine("All blogs in database:");
            foreach (var blog in db.Blogs.Include(b => b.Tenant))
                Console.WriteLine($"{blog.Tenant.Name} : {blog.Url}");
        }
    }
}

public class BloggingContext : DbContext
{
    public static string TenantId = "'99d1ac31-4122-4db1-a899-ba4b73063f14'";

    public DbSet<Tenant> Tenants { get; set; }
    public DbSet<Blog> Blogs { get; set; }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        optionsBuilder.UseSqlServer(@"Server=(localdb)\mssqllocaldb;Database=EFGetStarted.ConsoleApp.NewDb;Trusted_Connection=True;");
    }

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        base.OnModelCreating(modelBuilder);

        modelBuilder.Entity<Blog>(e =>
        {
            e.HasKey(nameof(Blog.TenantId), nameof(Blog.Id));
            e.Property(p => p.Id).HasDefaultValueSql("NEWSEQUENTIALID()");
            e.Property(p => p.TenantId).HasDefaultValueSql(TenantId); // fixed id for example, otherwise per session_context
        });
        modelBuilder.Entity<Tenant>(e =>
        {
            e.HasKey(nameof(Tenant.Id));
            e.Property(p => p.Id).HasDefaultValueSql(TenantId); // fixed id for example, otherwise per session_context
        });
    }
}

public class Blog
{
    public Guid Id { get; set; }
    public Guid TenantId { get; set; }
    public Tenant Tenant { get; set; }
    public string Url { get; set; }
}

public class Tenant
{
    public Guid Id { get; set; }
    public string Name { get; set; }
}
gdoron commented 8 years ago

Because of query performance reasons I added the tenant's Id to each primary key so all primary keys consist of 2 properties: Id + TenantId

Curious to know how would that improve the performance...

audacity76 commented 8 years ago

@gdoron Because every query on any table needs the TenantId (it's part of my row level security)

http://dba.stackexchange.com/questions/98118/composite-primary-key-in-multi-tenant-sql-server-database

gdoron commented 8 years ago

I'm still not convinced it's the easiest and performant way(why does it have to be in the key, and not a normal column), but I guess it is context based. Good luck!

ajcvickers commented 8 years ago

@audacity76 It doesn't make sense for Blog.TenantId to be both an FK and be store generated. When a property is an FK, then the value it has is driven by the property at the other end of the relationship--Tenant.Id in this case. Using store generated keys and using your example classes, this would mean Tenant.Id would be marked as store generated, but Blog.TenantId would not. Then, to insert a new Tenant and a new Blog the following would happen:

This ensures that the FK in the Blog matches the Id in the Tenant and that the entities are therefore related.