Property conversions are lost on base type is subtype is registered to model afterwards after upgrade to EFCore 8

[zlepper]

File a bug

If you register a property converter on a type, and then register a subclass of that type, the property converter is lost. (At least for primitive collections, i have no tested complex types). Instead EF uses the new automatic json conversion available for collections of primitive types.

This used to work fine in EFCore 7.

This is currently breaking all existing data we have that was written to the database with EF 7 and below, which is quite bad. Unit tests did not catch this on our side when we upgraded as they always generate new data and ended up just using the default json converter.

Include your code

Db context:

using System.Diagnostics;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.Metadata;

namespace something;

public class MyDbContext : DbContext
{
    public DbSet<MyBaseClass> MyThings { get; set; } = null!;

    private Action<ModelBuilder> _modelBuilderAction;

    public MyDbContext(DbContextOptions options, Action<ModelBuilder> modelBuilderAction) : base(options)
    {
        _modelBuilderAction = modelBuilderAction;
    }

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

    public static void Works(ModelBuilder modelBuilder)
    {
        
        modelBuilder.Entity<MyConcreteClass>();
        modelBuilder.Entity<MyBaseClass>(myBaseClass =>
        {
            myBaseClass.Property(c => c.SomeGuids).HasConversion(gs => string.Join(',', gs),
                s => s.Split(',', StringSplitOptions.RemoveEmptyEntries).Select(Guid.Parse).ToList());
        });
        var prop = modelBuilder.Model.FindEntityType(typeof(MyBaseClass))!.FindProperty(nameof(MyBaseClass.SomeGuids))!;
        ConversionBrokeException.ThrowIfPrimitive(prop);

        throw new Exception("Worked: " + prop.DeclaringType.Model.ToDebugString(MetadataDebugStringOptions.LongDefault));
    }

    public static void Broken(ModelBuilder modelBuilder)
    {
        
        modelBuilder.Entity<MyBaseClass>(myBaseClass =>
        {
            myBaseClass.Property(c => c.SomeGuids).HasConversion(gs => string.Join(',', gs),
                s => s.Split(',', StringSplitOptions.RemoveEmptyEntries).Select(Guid.Parse).ToList());
        });
        var prop = modelBuilder.Model.FindEntityType(typeof(MyBaseClass))!.FindProperty(nameof(MyBaseClass.SomeGuids))!;
        ConversionBrokeException.ThrowIfPrimitive(prop);

        modelBuilder.Entity<MyConcreteClass>();
        ConversionBrokeException.ThrowIfPrimitive(prop);
    }
}

public abstract class MyBaseClass
{
    public int Id { get; set; }
    
    public List<Guid> SomeGuids { get; set; } = new();
}

public class MyConcreteClass : MyBaseClass
{
    
}

public class ConversionBrokeException : Exception
{
    public ConversionBrokeException(string? message) : base(message)
    {
    }

    public static void ThrowIfPrimitive(IMutableProperty prop)
    {
        if (prop.IsPrimitiveCollection)
        {
            throw new ConversionBrokeException(prop.DeclaringType.Model.ToDebugString(MetadataDebugStringOptions.LongDefault));
        }
    }
}

Unit tests

using Microsoft.EntityFrameworkCore;
using NUnit.Framework;

namespace something;

[TestFixture]
public class MyTests
{
    [Test]
    public void Works()
    {
        Assert.That(() =>
        {
            var opts = new DbContextOptionsBuilder<MyDbContext>().UseSqlServer("").Options;
            var ctx = new MyDbContext(opts, MyDbContext.Works);
            ctx.MyThings.ToList();
        }, Throws.Exception.Not.TypeOf<ConversionBrokeException>());
    }
    
// This tests fails as the ConversionBrokeException is thrown.
    [Test]
    public void Broken()
    {
        Assert.That(() =>
        {
            var opts = new DbContextOptionsBuilder<MyDbContext>().UseSqlServer("").Options;
            var ctx = new MyDbContext(opts, MyDbContext.Broken);
            ctx.MyThings.ToList();
        }, Throws.Exception.Not.TypeOf<ConversionBrokeException>());
    }
}

Zip with entire project should be attached also
efwtf.zip

Include stack traces

I'm throwing the exceptions myself in the example code to make it clear that stuff has broken.

Include provider and version information

EF Core version:8.0.0
Database provider:Microsoft.EntityFrameworkCore.SqlServer
Target framework: net8.0
Operating system: Windows 10
IDE:

JetBrains Rider 2023.2.3
Build #RD-232.10203.29, built on November 1, 2023
Licensed to XXX
You have a perpetual fallback license for this version.
Subscription is active until September 19, 2024.
Runtime version: 17.0.8.1+7-b1000.32 amd64
VM: OpenJDK 64-Bit Server VM by JetBrains s.r.o.
Windows 10.0
.NET Core v7.0.7 x64 (Server GC)
GC: G1 Young Generation, G1 Old Generation
Memory: 12000M
Cores: 64
Registry:
    vcs.empty.toolwindow.show=false
    eslint.additional.file.extensions=svelte
    ide.new.project.model.index.case.sensitivity=true
    database.show.search.tab=false
    http.client.file.variables.available=true

Non-Bundled Plugins:
    org.jetbrains.plugins.go-template (232.9921.89)
    Batch Scripts Support (1.0.13)
    com.github.copilot (1.4.2.3864)
    com.intellij.plugin.adernov.powershell (2.3.0)
    org.intellij.plugins.hcl (232.8660.88)
    org.toml.lang (232.8660.88)
    idea.plugin.protoeditor (232.9559.10)
    com.intellij.kubernetes (232.10203.2)
    com.intellij.bigdatatools.core (232.10203.10)
    dev.blachut.svelte.lang (232.9921.36)
    verify-rider (2023.2.0)

Additional information

Working model:

Model: 
  EntityType: MyBaseClass Abstract
    Properties: 
      Id (int) Required PK AfterSave:Throw ValueGenerated.OnAdd
      Discriminator (no field, string) Shadow Required AfterSave:Throw
        Annotations: 
          AfterSaveBehavior: Throw
          ValueGeneratorFactoryType: Microsoft.EntityFrameworkCore.ValueGeneration.DiscriminatorValueGeneratorFactory
      SomeGuids (List<Guid>) Required
        Annotations: 
          ProviderClrType: 
          ValueConverter: Microsoft.EntityFrameworkCore.Storage.ValueConversion.ValueConverter`2[System.Collections.Generic.List`1[System.Guid],System.String]
    Keys: 
      Id PK
    Annotations: 
      DiscriminatorProperty: Discriminator
      DiscriminatorValue: MyBaseClass
      Relational:TableName: MyThings
      RelationshipDiscoveryConvention:NavigationCandidates: System.Collections.Immutable.ImmutableSortedDictionary`2[System.Reflection.PropertyInfo,System.ValueTuple`2[System.Type,System.Nullable`1[System.Boolean]]]
  EntityType: MyConcreteClass Base: MyBaseClass
    Annotations: 
      DiscriminatorValue: MyConcreteClass
      RelationshipDiscoveryConvention:NavigationCandidates: System.Collections.Immutable.ImmutableSortedDictionary`2[System.Reflection.PropertyInfo,System.ValueTuple`2[System.Type,System.Nullable`1[System.Boolean]]]
Annotations: 
  BaseTypeDiscoveryConvention:DerivedTypes: System.Collections.Generic.Dictionary`2[System.Type,System.Collections.Generic.List`1[Microsoft.EntityFrameworkCore.Metadata.IConventionEntityType]]
  NonNullableConventionState: System.Reflection.NullabilityInfoContext
  ProductVersion: 8.0.0
  Relational:MaxIdentifierLength: 128
  RelationshipDiscoveryConvention:InverseNavigationCandidates: System.Collections.Generic.Dictionary`2[System.Type,System.Collections.Generic.SortedSet`1[System.Type]]
  SqlServer:ValueGenerationStrategy: IdentityColumn

Broken model:

Model: 
  EntityType: MyBaseClass Abstract
    Properties: 
      Id (int) Required PK AfterSave:Throw ValueGenerated.OnAdd
      Discriminator (no field, string) Shadow Required AfterSave:Throw
        Annotations: 
          AfterSaveBehavior: Throw
          ValueGeneratorFactoryType: Microsoft.EntityFrameworkCore.ValueGeneration.DiscriminatorValueGeneratorFactory
      SomeGuids (List<Guid>) Required Element type: Guid Required
        Annotations: 
          ElementType: Element type: Guid Required
          ProviderClrType: 
          ValueConverter: 
          ValueConverterType: 
    Keys: 
      Id PK
    Annotations: 
      DiscriminatorProperty: Discriminator
      DiscriminatorValue: MyBaseClass
      Relational:TableName: MyThings
      RelationshipDiscoveryConvention:NavigationCandidates: System.Collections.Immutable.ImmutableSortedDictionary`2[System.Reflection.PropertyInfo,System.ValueTuple`2[System.Type,System.Nullable`1[System.Boolean]]]
  EntityType: MyConcreteClass Base: MyBaseClass
    Annotations: 
      DiscriminatorValue: MyConcreteClass
      RelationshipDiscoveryConvention:NavigationCandidates: System.Collections.Immutable.ImmutableSortedDictionary`2[System.Reflection.PropertyInfo,System.ValueTuple`2[System.Type,System.Nullable`1[System.Boolean]]]
Annotations: 
  BaseTypeDiscoveryConvention:DerivedTypes: System.Collections.Generic.Dictionary`2[System.Type,System.Collections.Generic.List`1[Microsoft.EntityFrameworkCore.Metadata.IConventionEntityType]]
  NonNullableConventionState: System.Reflection.NullabilityInfoContext
  ProductVersion: 8.0.0
  Relational:MaxIdentifierLength: 128
  RelationshipDiscoveryConvention:InverseNavigationCandidates: System.Collections.Generic.Dictionary`2[System.Type,System.Collections.Generic.SortedSet`1[System.Type]]
  SqlServer:ValueGenerationStrategy: IdentityColumn

[zlepper]

Just a "fun" thought about this: this can lead to "data corruption" even when fixed. If someone manages to insert data into the database while having the bug occurring they won't be able to read it once the issue has been fixed and will have to correct the data using direct SQL scripts or possibly even a custom application, depending on the exact converter they have configured.