Model configuration: FK exception with some combinations of property names and inheritance

[HappyNomad]

I'm mapping a complex domain model and encountered the exception, "The property 'ID' cannot be part of a foreign key on 'T' because it is contained in a key defined on a base entity type." Looks like the throwing of this exception was introduced in #2837.

At first I thought this was due to either a limitation of TPH mapping in general, or the current EF core implementation specifically. But now I think it's a bug in EF Core. It was difficult to reproduce in isolation since it obscurely depends on property names, too.

Trying to create a database using this context and model will reproduce the issue:

public class BloggingContext : DbContext
{
    public BloggingContext( DbContextOptions<BloggingContext> options )
        : base( options ) { }

    public DbSet<L> Ls { get; set; }
    public DbSet<PBase> PBases { get; set; }
    public DbSet<P> Ps { get; set; }
    public DbSet<Q> Qs { get; set; }
    public DbSet<T> Ts { get; set; }
}

public class L
{
    public int ID { get; set; }
    public IList<T> Ts { get; set; }
}

public abstract class PBase
{
    public int ID { get; set; }
}

public class P : PBase
{
}

public class Q : PBase
{
}

public class T : P
{
    public P P { get; set; }
    public Q F { get; set; }
}

A workaround for this example is to rename the context's "Ls" DbSet property to "Zs". Unfortunately, I've already come across a case in my real domain model where this simple hack doesn't help. That is, it removes the exception for one class but that, in turn, causes the same exception to occur for another class. Removing one of the properties but not the other doesn't remove the exception, either.

[HappyNomad]

I've updated my original comment with a model that reproduces the issue in isolation. It was difficult to whittle down this small model since reproducing the issue depends on class and property names.

[AndriySvyryd]

It shouldn't depend on the naming as you've described, so there could be a bug in this particular scenario, it should still throw if Ls is renamed to Zs.
In general it's a limitation of TPH, you cannot have an FK from T.ID to P.ID, because they are mapped to the same table and ID needs to be unique. The workaround is to use a different property for the FK. It can be a shadow property:

modelbuilder.Entity<T>().Property<int>("PId");
modelbuilder.Entity<T>().HasOne(t => t.P).WithOne().HasForeignKey<T>("PId");

[HappyNomad]

Okay, so now I understand the default behavior is as stated here: "In a one-to-one relationship, the primary key acts additionally as a foreign key and there is no separate foreign key column for either table.".

But then why not do the correction automatically similar to how this happens automatically, instead of throwing the exception and requiring extra code?

Anyway, please explain why the exact same exception is thrown even for this model, where T only participates in a one-to-many relationship. Is it a bug too?

public class BloggingContext : DbContext
{
    public BloggingContext( DbContextOptions<BloggingContext> options )
        : base( options ) { }

    public DbSet<A> As { get; set; }
}

public class A
{
    public int ID { get; set; }
    public L L1 { get; set; }
    public IList<PBase> L2 { get; set; }
}

public class L
{
    public int ID { get; set; }
    public IList<T> Ts { get; set; }
}

public class T : P { }

public class P : PBase { }

public class Q : PBase { }

public abstract class PBase
{
    public int ID { get; set; }
    public string Stuff { get; set; }
}

This example has a similar strange workaround: flip the L1 and L2 names and then the exception disappears.

[AndriySvyryd]

Fixed in 66c4c79

[HappyNomad]

Thanks for the fix here and on #6814. I see both are on the "feature/1.2.0" branch. I'm now using MyGet's ASP.NET Core "dev" branch feed with this version configured:

    "Microsoft.EntityFrameworkCore.SqlServer": "1.1.0-preview2-*"

So I can't benefit from your changes until after the 1.1.0 release, at which time I'll be able to indicate the version as 1.2.0-preview1-*, right? I'd just like to verify that I correctly understand, so I don't wait unnecessarily. I looked at the "aspnetcore-feature-work" feed but don't see EF-Core (not sure I should use that feed even if it were there, though).

[AndriySvyryd]

@HappyNomad Correct. Hopefully you won't need to wait long.

[divega]

I looked at the "aspnetcore-feature-work" feed but don't see EF-Core (not sure I should use that feed even if it were there, though).

@pranavkm @Eilon any idea why the build output of our feature/1.2.0 branch is not landing in the aspnetcore-feature-work feed? I was under the impression it was automatic 😄

[pranavkm]

@divega unfortunatelty it's not automagic. You have to tell the build to push the packages. Here's what Razor does to push - https://github.com/aspnet/Razor/blob/feature/razor-pages/makefile.shade.

[divega]

cc @anpete about what you asked, see @pranavkm's comment above. Feel free to fix it, or we can wait until we marge feature/1.2.0 into dev.

[Eilon]

You are free to merge 1.2.0 changes into dev - the branch is open.

[HappyNomad]

But then why not do the correction automatically similar to how this happens automatically, instead of throwing the exception and requiring extra code?

@AndriySvyryd To clarify, did you implement this suggestion? I'm using version 1.2.0-preview* and it seems to be working that way. The model builds fine even without the OnModelCreating workaround you suggested.

Also to clarify, you suggested the workaround:

modelbuilder.Entity<T>().Property<int>("PId");
modelbuilder.Entity<T>().HasOne(t => t.P).WithOne().HasForeignKey<T>("PId");

But I believe it should instead be:

modelbuilder.Entity<P>().Property<int>("PId");
modelbuilder.Entity<T>().HasOne(t => t.P).WithOne().HasForeignKey<P>("PId");

Right?

[AndriySvyryd]

@HappyNomad Yes, the fix is to choose FK properties that are compatible with the base type if you haven't specified any explicitly, so no extra configuration would be needed.

[AndriySvyryd]

Both workarounds are valid, it depends on what type you want to be the dependent one.

[HappyNomad]

@AndriySvyryd I looked at the generated model and it's reversing the principle/dependent roles from what I need. I see this issue with other one-to-one relationships in my model, too. My model classes lack navigation properties that point from dependent to principle. I would like the model builder to leverage my consistent approach to navigation to correctly determine principle/dependent roles.

The documentation says, "EF will choose one of the entities to be the dependent based on its ability to detect a foreign key property." In the absence of a FK property, please augment this behavior such that, when only one end of a relationship has a navigation property, then that property is assumed to be on the principle entity pointing to the dependent. This won't always be true for everybody, but I think it will be true more often than the reverse. If someone strongly disagrees, then at least provide a *Mode property on ModelBuilder to toggle this behavior.

Or is there at least some attribute I can add to my model classes' properties, so that the model builder generates as I need, without the extra configuration in OnModelCreating? I'd prefer something concise like the [Part] idea I proposed. I tried the existing, more verbose approach to add foreign keys to the dependent entities and apply [ForeignKey] to the principle entity's navigation properties, but that doesn't appear to be working for multiple navigations to the same type.

For the time being at least, I'll define my own [Part] class and then use reflection in the OnModelCreating method to configure the model with the fluent API. In addition to the above-mentioned configuration, I'll also mark all aggregations, including those of one-to-many relationships, as required.

[AndriySvyryd]

@HappyNomad We actually tried to use the proposed heuristic in addition to others before the 1.0.0 release, but it became harder to explain and understand how EF chooses the dependent side, while it didn't improve the success rate that much, as many users have different expectations.
Unfortunately there's currently no public way of doing what you want, but it should be handled by Custom Conventions (#214)