Improve conflicting foreign keys exception to show which navigations each foreign key is associated with

[douglasg14b]

When I use [ForeignKey] on a dependent entity, and the fluent API on the parent with a backing field to represent the inverse navigation the below exception is thrown.

Partially Works without [ForeignKey]:

If I remove the [ForeignKey("AttributeId")] from AttributeValue, the exception goes away, but the Attribute property is always null. It's never filled in when the entity is loaded.

Works by Convention:

If I completely drop all explicit configurations (Fluent API or Attributes) and let EF sort it out via convention, it works as expected. No exceptions, and the Attribute field on AttributeValue gets filled in when the entity is loaded.

Works if not using a backing field:

If I drop the use of a backing field, and set this up like the Blog/Post examples using explicit attributes (ie. [ForeignKey]) it also fills in the navigational property when retrieving the entity from the DB. Though, this defeats the purpose of being defensive with the AttributeValue collection.

Exception message:

'The relationship from 'AttributeValue.Attribute' to 'Attribute' with foreign key properties {'AttributeId' : int} cannot target the primary key {'Id' : int} because it is not compatible. Configure a principal key or a set of compatible foreign key properties for this relationship.'

Steps to reproduce

Below code causes this:

Attribute:

    public class Attribute
    {
        [Key]
        [DatabaseGenerated(DatabaseGeneratedOption.Identity)]
        public int Id { get; private set; }
        private HashSet<AttributeValue> _values;
        public IEnumerable<AttributeValue> Values => _values?.ToList();

        //[Snip]
}

AttributeValue:

    public class AttributeValue : IEntity
    {
        [Key]
        [DatabaseGenerated(DatabaseGeneratedOption.Identity)]
        public int Id { get; private set; }
        public string Value { get; private set; }

        public int AttributeId { get; private set; }
        [ForeignKey("AttributeId")]
        public Attribute Attribute { get; private set; }

        //[Snip]
}

AttributeConfiguration:

    public class AttributeConfiguration : IEntityTypeConfiguration<Attribute>
    {
        public void Configure(EntityTypeBuilder<Attribute> builder)
        {
            builder.HasMany(x => x.Values)
                .WithOne()
                .HasForeignKey(x => x.AttributeId);

            builder.Metadata
                .FindNavigation(nameof(Attribute.Values))
                .SetPropertyAccessMode(PropertyAccessMode.Field);
        }
    }

Further technical details

EF Core version: 2.2.4 Database Provider: InMemory Operating system: Server 2016 IDE: Visual Studio 2019

[douglasg14b]

This has a tentative relation to some of the code/conversation in this issue: https://github.com/aspnet/EntityFrameworkCore/issues/10800

[ajcvickers]

@douglasg14b The issue here is that this code:

public int AttributeId { get; private set; }
[ForeignKey("AttributeId")]
public Attribute Attribute { get; private set; }

tells EF that the navigation property Attribute should use the AttributeId property for the FK.

But then this code:

builder.HasMany(x => x.Values)
    .WithOne()
    .HasForeignKey(x => x.AttributeId);

tells EF that the same FK property should be used for a relationship that explicitly does not use that navigation property, since no navigation property is specified in the WithOne call.

Changing it to be consistent with the configuration supplied by the attributes:

builder.HasMany(x => x.Values)
    .WithOne(x => x.Attribute)
    .HasForeignKey(x => x.AttributeId);

makes this work.

Note for triage: the exception message is not very helpful here.

[douglasg14b]

@ajcvickers

Oh, complete PEBKAC on my end! Thanks for the clarification, I completely missed that.

I'll leave this open for comment on the error message, which could provide better clarification in an instance such as this.

[ajcvickers]

Triage: Putting this on the backlog to make the exception message better.

[ajcvickers]

On latest daily, this doesn't throw, but instead logs the warning:

warn: 10/18/2021 12:35:43.733 CoreEventId.ShadowForeignKeyPropertyCreated[10625] (Microsoft.EntityFrameworkCore.Model.Validation)
      The foreign key property 'AttributeValue.AttributeId1' was created in shadow state because a conflicting property with the simple name 'AttributeId' exists in the entity type, but is either not mapped, is already used for another relationship, or is incompatible with the associated primary key type.
See https://aka.ms/efcore-relationships for information on mapping relationships in EF Core.

And generates the model:

Model: 
  EntityType: Attribute
    Properties: 
      Id (int) Required PK AfterSave:Throw ValueGenerated.OnAdd
    Navigations: 
      Values (_values, IEnumerable<AttributeValue>) Collection ToDependent AttributeValue PropertyAccessMode.Field
    Keys: 
      Id PK
  EntityType: AttributeValue
    Properties: 
      Id (int) Required PK AfterSave:Throw ValueGenerated.OnAdd
      AttributeId (int) Required FK Index
      AttributeId1 (no field, int?) Shadow FK Index
      AttributeId2 (no field, int) Shadow Required
      Value (string)
    Navigations: 
      Attribute (Attribute) ToPrincipal Attribute
    Keys: 
      Id PK
    Foreign keys: 
      AttributeValue {'AttributeId'} -> Attribute {'Id'} ToDependent: Values Cascade
      AttributeValue {'AttributeId1'} -> Attribute {'Id'} ToPrincipal: Attribute ClientSetNull
    Indexes: 
      AttributeId 
      AttributeId1 

It's not clear to me wht the AttributeId2 shadow property exists.

[AndriySvyryd]

On latest daily, this doesn't throw, but instead logs the warning:

We can separate the warning into specific messages for each listed reason with additional information

It's not clear to me wht the AttributeId2 shadow property exists.

Artifact/bug of the way FK properties are reuniquified to avoid holes in numbering, should be fixed by layering

[ajcvickers]

Notes from triage:

• Leaving this issue open to check if there are other cases that still need a better exception, and to improve the existing message.
• Filed #26412 for the bad model