Scaffold tool can create an inverse property with a similar name to a column property and get confused

[conficient]

If a table has a foreign key column and a value column with a similar name, then then scaffold tool can create a model which confuses the model builder as they have the same property name if case isn't matched.

Detail

I have a database with a CPSorder table with a column CPSChargePartner column (smallmoney, nullable).

This table also has a foreign key to a Partners table with the foreign key column called CPSchargePartnerID.

In earlier versions of the dotnet ef dbcontext scaffold command had no issues. Recently I upgraded my code to use EF Core tools 5.0.8 and rebuilt the context (full command below):

dotnet ef dbcontext scaffold "Server=xxx;Database=AnvilFGDEV;Trusted_Connection=True;" Microsoft.EntityFrameworkCore.SqlServer --force  --namespace AnvilDB --context AnvilDBcontext --context-dir Context --data-annotations --use-database-names --output-dir Tables >_build_db.log

This generated everything without issues. However when I tried to use the context it crashed with a NullReferenceException somewhere in the create model process:

Message: 
    Test method AnvilDB.Tests.AnvilDBcontext_Table_Tests.AnvilFGDdev threw exception: 
    System.NullReferenceException: Object reference not set to an instance of an object.

  Stack Trace: 
    InversePropertyAttributeConvention.ConfigureInverseNavigation(IConventionEntityTypeBuilder entityTypeBuilder, MemberInfo navigationMemberInfo, IConventionEntityTypeBuilder targetEntityTypeBuilder, InversePropertyAttribute attribute)
    InversePropertyAttributeConvention.Process(IConventionEntityTypeBuilder entityTypeBuilder, MemberInfo navigationMemberInfo, Type targetClrType, InversePropertyAttribute attribute)
    InversePropertyAttributeConvention.ProcessEntityTypeAdded(IConventionEntityTypeBuilder entityTypeBuilder, MemberInfo navigationMemberInfo, Type targetClrType, InversePropertyAttribute attribute, IConventionContext`1 context)
    NavigationAttributeConventionBase`1.ProcessEntityTypeAdded(IConventionEntityTypeBuilder entityTypeBuilder, IConventionContext`1 context)
    ImmediateConventionScope.OnEntityTypeAdded(IConventionEntityTypeBuilder entityTypeBuilder)
    OnEntityTypeAddedNode.Run(ConventionDispatcher dispatcher)
    DelayedConventionScope.Run(ConventionDispatcher dispatcher)
    ConventionBatch.Run()
    ConventionBatch.Dispose()
    ImmediateConventionScope.OnModelInitialized(IConventionModelBuilder modelBuilder)
    <26 more frames...>
    DbContext.get_InternalServiceProvider()
    DbContext.get_DbContextDependencies()
    DbContext.get_Model()
    InternalDbSet`1.get_EntityType()
    InternalDbSet`1.CheckState()
    InternalDbSet`1.get_EntityQueryable()
    IEnumerable<TEntity>.GetEnumerator()
    List`1.ctor(IEnumerable`1 collection)
    Enumerable.ToList[TSource](IEnumerable`1 source)
    AnvilDBcontext_Table_Tests.AnvilFGDdev() line 21

Cue a lot of headscratching and also referring to this issue. I then tried updating to EF Core 6 preview and trying to run the generated context in .NET 6 preview: this resulted in this:

 Message: 
    Test method TestContext.AnvilFGDEV_Test.ReadTablesAsync threw exception: 
    System.Collections.Generic.KeyNotFoundException: The given key 'System.Nullable`1[System.Decimal] CPSChargePartner' was not present in the dictionary.

  Stack Trace: 
    Immutable.dll:token 0x6000490+0x34
    EntityFrameworkCore.dll:token 0x6001591+0x4c

Cause

So the inverse property name should be CPSchargePartner but CPSChargePartner (the column) was being matched.

When the scaffold tool ran, it created the following in CPSOrder.cs:

        [Column(TypeName = "smallmoney")]
        public decimal? CPSChargePartner { get; set; }

        // other lines skipped

        [ForeignKey(nameof(CPSchargePartnerID))]
        [InverseProperty(nameof(Partner.CPSorders))]
        public virtual Partner CPSchargePartner { get; set; }

In the related table Partners.cs we have:

        [InverseProperty(nameof(CPSorder.CPSchargePartner))]
        public virtual ICollection<CPSorder> CPSorders { get; set; }

and in the context:

        entity.HasOne(d => d.CPSchargePartner)
            .WithMany(p => p.CPSorders)
            .HasForeignKey(d => d.CPSchargePartnerID)
            .HasConstraintName("FK_CPSorders_Partners");

So although everything looks okay, when the model is being created it tries to use the property CPSChargePartner which isn't a foreign key and so wasn't in the map. If the mapping/lookup isn't case sensitive perhaps the scaffold tool should refuse to create possible conflicting properties?

I was able to work around this by renaming the property so that it would not get picked up:

        [Column("CPSChargePartner", TypeName = "smallmoney")]
        public decimal? CPSChargeForPartner { get; set; }

[ajcvickers]

@conficient Can you please post the SQL to create the database tables involved so we can try to reproduce this.

[conficient]

@conficient Can you please post the SQL to create the database tables involved so we can try to reproduce this.

I've created a repo with the SQL and the generated code for you: https://github.com/conficient/EFscaffoldBug

Trimmed out all but the two tables and the relevant columns and the bug is still evident.

[smitpatel]

This is model building bug.

efcore/src/EFCore/Metadata/Conventions/InversePropertyAttributeConvention.cs

Lines 80 to 81 in 9fa54fe

	var inverseNavigationPropertyInfo = targetEntityType.GetRuntimeProperties().Values
	.FirstOrDefault(p => string.Equals(p.GetSimpleMemberName(), attribute.Property, StringComparison.OrdinalIgnoreCase));

Not sure why that line is comparing property names ignoring case.
cc: @AndriySvyryd

[AndriySvyryd]

Been like that since the start: 81b1be1#diff-4c3db0c4694e7ac035dd26ec3393ed3e7bd9fd10225d4c3b50129fcaa1dda962R40

Probably to match EF6 behavior.

[smitpatel]

@dotnet/efteam What is the suggested fix here? Should we make C# properties in generated entity type unique ignoring casing?

[ajcvickers]

Note from triage: fix by doing a case-sensitive match first, followed by a case-insensitive match, in the convention.