Changing a DbSet<> property name can create incorrect TPH foreign keys in model

[LeroyK]

Changing a DbSet<> property name can result in incorrect TPH foreign keys in the model.

Just run the example code below with the following instructions.

The referencing foreign keys are captured at the first line of OnModelCreating (set a breakpoint), before any custom model configuration is done.

When running the code with the DbSet<Packs> named as Packs" everything runs fine. OnModelCreating succeeds and the resulting referencing foreign keys on PetOwner are (as expected):

modelBuilder.Entity<PetOwner>().Metadata.GetReferencingForeignKeys()
Count = 3
    [0]: {ForeignKey: Cat {'PetOwnerId'} -> PetOwner {'Id'} ToDependent: Cats ToPrincipal: PetOwner}
    [1]: {ForeignKey: Dog {'PetOwnerId'} -> PetOwner {'Id'} ToDependent: Dogs ToPrincipal: PetOwner}
    [2]: {ForeignKey: Pack {'PetOwnerId'} -> PetOwner {'Id'} ToPrincipal: PetOwner}

Now if we rename Packs to APacks, the referencing foreign keys on PetOwner are:

modelBuilder.Entity<PetOwner>().Metadata.GetReferencingForeignKeys()
Count = 4
    [0]: {ForeignKey: Pack {'PetOwnerId'} -> PetOwner {'Id'} ToPrincipal: PetOwner}
    [1]: {ForeignKey: Pet {'PetOwnerId'} -> PetOwner {'Id'} ToPrincipal: PetOwner}
    [2]: {ForeignKey: Cat {'PetOwnerId1'} -> PetOwner {'Id'} ToDependent: Cats}
    [3]: {ForeignKey: Dog {'PetOwnerId1'} -> PetOwner {'Id'} ToDependent: Dogs}

This is incorrect, foreign key [1] should not have been created. Furthermore, OnModelCreating now fails with the exception below.

Exception message:
The navigation property 'Dogs' cannot be added to the entity type 'PetOwner' because its CLR type 'List<Dog>' does not match the expected CLR type 'Pet'.

Stack trace:
   at Microsoft.EntityFrameworkCore.Metadata.Internal.Navigation.IsCompatible(MemberInfo navigationProperty, Type sourceClrType, Type targetClrType, Nullable`1 shouldBeCollection, Boolean shouldThrow)
   at Microsoft.EntityFrameworkCore.Metadata.Internal.InternalRelationshipBuilder.IsCompatible(MemberInfo navigationProperty, Boolean pointsToPrincipal, Type dependentType, Type principalType, Boolean shouldThrow, Nullable`1& shouldBeUnique)
   at Microsoft.EntityFrameworkCore.Metadata.Internal.InternalRelationshipBuilder.CanSetNavigations(Nullable`1 navigationToPrincipal, Nullable`1 navigationToDependent, EntityType principalEntityType, EntityType dependentEntityType, Nullable`1 configurationSource, Boolean shouldThrow, Boolean overrideSameSource, Nullable`1& shouldInvert, Nullable`1& shouldBeUnique, Boolean& removeOppositeNavigation, Boolean& removeConflictingNavigations)
   at Microsoft.EntityFrameworkCore.Metadata.Internal.InternalRelationshipBuilder.HasNavigations(Nullable`1 navigationToPrincipal, Nullable`1 navigationToDependent, EntityType principalEntityType, EntityType dependentEntityType, ConfigurationSource configurationSource)
   at Microsoft.EntityFrameworkCore.Metadata.Internal.InternalRelationshipBuilder.HasNavigations(Nullable`1 navigationToPrincipal, Nullable`1 navigationToDependent, ConfigurationSource configurationSource)
   at Microsoft.EntityFrameworkCore.Metadata.Internal.InternalRelationshipBuilder.HasNavigation(MemberInfo property, Boolean pointsToPrincipal, ConfigurationSource configurationSource)
   at Microsoft.EntityFrameworkCore.Metadata.Builders.ReferenceNavigationBuilder.WithManyBuilder(MemberIdentity collection)
   at Microsoft.EntityFrameworkCore.Metadata.Builders.ReferenceNavigationBuilder.WithManyBuilder(MemberInfo navigationMemberInfo)
   at Microsoft.EntityFrameworkCore.Metadata.Builders.ReferenceNavigationBuilder`2.WithMany(Expression`1 navigationExpression)
   at EFCoreBugExample1.ExampleContext.OnModelCreating(ModelBuilder modelBuilder) in C:\TestProjects\EFCoreBugExample1\EFCoreBugExample1\Program.cs:line 49
   at Microsoft.EntityFrameworkCore.Infrastructure.ModelCustomizer.Customize(ModelBuilder modelBuilder, DbContext context)
   at Microsoft.EntityFrameworkCore.Infrastructure.ModelSource.CreateModel(DbContext context, IConventionSetBuilder conventionSetBuilder)
   at Microsoft.EntityFrameworkCore.Infrastructure.ModelSource.<>c__DisplayClass5_0.<GetModel>b__1()
   at System.Lazy`1.ViaFactory(LazyThreadSafetyMode mode)

Steps to reproduce

using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Design;
using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using System.ComponentModel.DataAnnotations.Schema;
using System.Threading.Tasks;

namespace EFCoreBugExample1
{
    class Program
    {
        public static string ConnectionString = "Server=.\\SQLEXPRESS;Database=ExampleDb;MultipleActiveResultSets=true";

        static async Task Main(string[] args)
        {
            var options = new DbContextOptionsBuilder()
                .UseSqlServer(ConnectionString)
                .Options;

            var db = new ExampleContext(options);
            db.ChangeTracker.Entries();
        }
    }

    public class ExampleDesignTimeDbContextFactory : IDesignTimeDbContextFactory<ExampleContext>
    {
        public ExampleContext CreateDbContext(string[] args) => new ExampleContext(
            new DbContextOptionsBuilder()
            .UseSqlServer(Program.ConnectionString)
            .Options);
    }

    public class ExampleContext : DbContext
    {
        public DbSet<Pack> APacks { get; set; }
        public DbSet<Pet> CountryLists { get; set; }
        public DbSet<PetOwner> PetOwners { get; set; }

        public ExampleContext(DbContextOptions options) : base(options) { }
        protected ExampleContext() { }

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.Entity<Pet>()
                .HasDiscriminator(x => x.Discriminator);

            modelBuilder
                .Entity<Dog>()
                .HasOne(x => x.PetOwner)
                .WithMany(x => x.Dogs);

            modelBuilder
                .Entity<Cat>()
                .HasOne(x => x.PetOwner)
                .WithMany(x => x.Cats);

            base.OnModelCreating(modelBuilder);
        }
    }

    public class PetOwner
    {

        [Key]
        public long Id { get; set; }

        [Timestamp]
        public byte[] RowVersion { get; set; }

        public List<Dog> Dogs { get; set; }

        public List<Cat> Cats { get; set; }
    }

    public class Pet
    {
        [Key]
        public long Id { get; set; }

        public string Discriminator { get; set; }

        [Timestamp]
        public byte[] RowVersion { get; set; }

        [ForeignKey(nameof(PetOwner))]
        public long PetOwnerId { get; set; }

        public PetOwner PetOwner { get; set; }
    }

    public class Cat : Pet
    {
        public int RemainingLives { get; set; }
    }

    public class Dog : Pet
    {
        [ForeignKey(nameof(Pack))]
        public long PackId { get; set; }

        public Pack Pack { get; set; }
    }

    public class Pack
    {
        [Key]
        public long Id { get; set; }

        [Timestamp]
        public byte[] RowVersion { get; set; }

        [ForeignKey(nameof(PetOwner))]
        public long PetOwnerId { get; set; }

        public PetOwner PetOwner { get; set; }

        public List<Dog> Dogs { get; set; }
    }
}

Further technical details

EF Core version: 3.0.0-preview7.19362.6
Database Provider:Microsoft.EntityFrameworkCore.SqlServer
Operating system: Windows 10 1903
IDE: Visual Studio 2019 16.3.0 Preview 1

[ajcvickers]

@LeroyK Does this also fail for you on 2.2.6?

@AndriySvyryd Repros for me on currently nightlies, but I'm not entirely sure what the expectation is in this case.

[smitpatel]

My thoughts: 2nd one is correct. Pet.PetOwner navigation is defined on base class which is part of model. And inverse navigations (Dogs/Cats) points to derived type. We cannot pair those up. Derived types cannot have same navigation as base type unless it is ignored on base type and added in derived type. Without paired up navigations, you would get what is in configuration 2.

Error is also valid bit difficult to understand. Trying to configure PetOwner navigation in derived type will get you navigation from base type and the inverse navigation is incompatible.

Does it work if you ignore the navigation in base class and then configure in derived types?

[LeroyK]

@LeroyK Does this also fail for you on 2.2.6?

No, this started happening after switching to 3.0. It worked fine in 2.2.6.

[LeroyK]

My thoughts: 2nd one is correct. Pet.PetOwner navigation is defined on base class which is part of model. And inverse navigations (Dogs/Cats) points to derived type. We cannot pair those up. Derived types cannot have same navigation as base type unless it is ignored on base type and added in derived type. Without paired up navigations, you would get what is in configuration 2.

I would say the first one is correct for the provider model, but not for the entity model. In OnModelCreating I am defining the navigations for Cat & Dog explicitely, not for Pet. When using Sql Server it should result in just one FK constraint. In 2.2.6 it created two, but that could be overcome by naming them the same using HasConstraintName("FK_Name").

Does it work if you ignore the navigation in base class and then configure in derived types?

This is actually what I want, but since the navigation is automatically created by EF Core when the name is APack (see OP), the navigation can't be ignored.

I actually got it to work by manually removing the foreign keys (not ideal):

1. Removing RelationshipDiscoveryConvention from ConventionSet.NavigationRemovedConventions, because this kept adding back the foreign keys after removing them.
2. Manually removing the foreign keys to PetOwner (from Pet, Cat & Dog).
3. Creating the navigations in OnModelCreating,

In any case, model creation should be deterministic and not be affected by naming of (probably implicitly order of?) DbSet<> property names.