dotnet / efcore

EF Core is a modern object-database mapper for .NET. It supports LINQ queries, change tracking, updates, and schema migrations.
https://docs.microsoft.com/ef/
MIT License
13.52k stars 3.13k forks source link

ComplexType.get_ConstructorBinding throws InvalidCastException trying to cast to IReadOnlyEntityType #32437

Open Timovzl opened 7 months ago

Timovzl commented 7 months ago

When I use ComplexProperty() with a custom value object, then as soon as I call Database.EnsureCreated(), EF throws an InvalidCastException in ModelValidator.ValidateFieldMapping.

Code

        var allocationBuilder = builder.ComplexProperty(x => x.Allocation);
        {
            allocationBuilder.Property(allocation => allocation.Target)
                .HasColumnName("TargetAllocation");

            allocationBuilder.Property(allocation => allocation.Min)
                .HasColumnName("MinAllocation");

            allocationBuilder.Property(allocation => allocation.Max)
                .HasColumnName("MaxAllocation");
        }

Exception

System.InvalidCastException : Unable to cast object of type 'Microsoft.EntityFrameworkCore.Metadata.Internal.ComplexType' to type 'Microsoft.EntityFrameworkCore.Metadata.IReadOnlyEntityType'.

Culprit

Looking at the source, ComplexType's ConstructorBinding getter casts the input to IReadOnlyEntityType, which appears to be an incorrect assumption.

Detailed exception

  Message: 
System.InvalidCastException : Unable to cast object of type 'Microsoft.EntityFrameworkCore.Metadata.Internal.ComplexType' to type 'Microsoft.EntityFrameworkCore.Metadata.IReadOnlyEntityType'.

  Stack Trace: 
<>c.<get_ConstructorBinding>b__42_0(ComplexType complexType)
NonCapturingLazyInitializer.EnsureInitialized[TParam,TValue](TValue& target, TParam param, Action`1 valueFactory)
ComplexType.get_ConstructorBinding()
ModelValidator.<ValidateFieldMapping>g__Validate|24_0(ITypeBase typeBase)
ModelValidator.<ValidateFieldMapping>g__Validate|24_0(ITypeBase typeBase)
ModelValidator.ValidateFieldMapping(IModel model, IDiagnosticsLogger`1 logger)
ModelValidator.Validate(IModel model, IDiagnosticsLogger`1 logger)
RelationalModelValidator.Validate(IModel model, IDiagnosticsLogger`1 logger)
SqlServerModelValidator.Validate(IModel model, IDiagnosticsLogger`1 logger)
ModelRuntimeInitializer.Initialize(IModel model, Boolean designTime, IDiagnosticsLogger`1 validationLogger)
<43 more frames...>
InfrastructureExtensions.GetService(IInfrastructure`1 accessor, Type serviceType)
InfrastructureExtensions.GetService[TService](IInfrastructure`1 accessor)
AccessorExtensions.GetService[TService](IInfrastructure`1 accessor)
DatabaseFacade.get_Dependencies()
DatabaseFacade.EnsureCreated()

Provider and version information

EF Core version: 8.0.0 Database provider: Microsoft.EntityFrameworkCore.SqlServer Target framework: .NET 8.0 Operating system: Windows 10 IDE: Visual Studio 2022 17.8.0

ajcvickers commented 7 months ago

This issue is lacking enough information for us to be able to fully understand what is happening. Please attach a small, runnable project or post a small, runnable code listing that reproduces what you are seeing so that we can investigate.

Timovzl commented 7 months ago

@ajcvickers Before I invest time building a minimal repro, I believe we can see a clear problem in the source code that warrants a closer look. Can you confirm that it is indeed weird and incorrect that a ComplexType instance is being cast to IReadOnlyEntityType? ComplexType does not implement that interface. Here is the exact location once more.

ajcvickers commented 7 months ago

@Timovzl Yes, but what causes that code path to be used?

Timovzl commented 7 months ago

@ajcvickers Ok, I've created a minimal repro.

The problem occurs when removing the ConstructorBindingConvention, such as to keep EF from binding to parameterized constructors or, in this case, because we use an IMaterializationInterceptor to handle all instance creation.

A few notes:

using System.Runtime.CompilerServices;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.Metadata.Conventions;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;

namespace ComplexTypeConstructorBindingDemo;

internal class Program
{
    public static void Main()
    {
        var builder = Host.CreateApplicationBuilder();

        builder.Services.AddPooledDbContextFactory<TestDbContext>(dbContext => dbContext
            .UseSqlServer(@"Data Source=(LocalDB)\MSSQLLocalDB;Integrated Security=True;Initial Catalog=ComplexTypeConstructorBindingDemo;", sqlServer => sqlServer.EnableRetryOnFailure())
            .AddInterceptors([new InstantiationInterceptor()]));

        using var host = builder.Build();
        host.Start();

        using var dbContext = host.Services.GetRequiredService<IDbContextFactory<TestDbContext>>().CreateDbContext();

        dbContext.Database.EnsureCreated();

        dbContext.Add(new Item(new Color(1, 1, 1)));
        dbContext.SaveChanges();
        var color = dbContext.Set<Item>().OrderBy(color => color.Id).Last();

        host.StopAsync().GetAwaiter().GetResult();
    }
}

#region DbContext Configuration

internal class TestDbContext(
    DbContextOptions<TestDbContext> options)
    : DbContext(options)
{
    protected override void ConfigureConventions(ModelConfigurationBuilder configurationBuilder)
    {
        base.ConfigureConventions(configurationBuilder);

        // Our IMaterializationInterceptor will create all entity instances
        configurationBuilder.Conventions.Remove(typeof(ConstructorBindingConvention)); // Outcomment this to one to avoid the bug (but run into another related to IMaterializationInterceptor passing RuntimeComplexType as an IEntityType parameter in an expression
    }

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

        modelBuilder.Entity<Item>(item =>
        {
            //item.OwnsOne(x => x.Color); // This works fine
            item.ComplexProperty(x => x.Color); // This encounters the bug

            item.HasKey(x => x.Id);
        });
    }
}

internal class InstantiationInterceptor : IMaterializationInterceptor
{
    public InterceptionResult<object> CreatingInstance(MaterializationInterceptionData materializationData, InterceptionResult<object> result)
    {
        // Avoid ctors when creating instances, to avoid re-running domain model validations
        var instance = RuntimeHelpers.GetUninitializedObject(materializationData.EntityType.ClrType);
        return InterceptionResult<object>.SuppressWithResult(instance);
    }
}

#endregion

#region Model

internal class Item // Entity
{
    public int Id { get; }
    public Color Color { get; }

    public Item(Color color)
    {
        this.Color = color;
    }

    private Item() // Reconstitution only
    {
    }
}

internal class Color // ValueObject
{
    public byte Red { get; private init; }
    public byte Blue { get; private init; }
    public byte Green { get; private init; }

    public Color(byte red, byte blue, byte green)
    {
        this.Red = red;
        this.Blue = blue;
        this.Green = green;
    }

    private Color() // Reconstitution only
    {
    }
}

#endregion
ajcvickers commented 7 months ago

@Timovzl What are you doing instead of using "ConstructorBindingConvention" to ensure each mapping has a constructor binding? I don't see anything in your repro code, which would leave the model without constructor bindings. This is not a valid state for the model to be in, although it worked for historical reasons for entity types.

In general, when removing a model building convention that provides some critical information on how to handle types in the model--in this case, how to create instances--it must be replaced by either explicit code or a different convention that ensures all required information is present.

Note for triage: consider adding model validation that all complex types have constructor bindings.

Timovzl commented 7 months ago

@ajcvickers That makes sense. I had left it out for simplicity, but I'm using the binding below. (Clearly it and the interceptor perform the same task, so either the interceptor could be omitted or the binding could just produce a dummy expression.)

internal sealed class UninitializedInstantiationConvention : IModelFinalizingConvention
{
    public void ProcessModelFinalizing(IConventionModelBuilder modelBuilder, IConventionContext<IConventionModelBuilder> context)
    {
        foreach (var entityType in modelBuilder.Metadata.GetEntityTypes())
        {
            if (entityType.ClrType.IsAbstract)
                continue;

#pragma warning disable EF1001 // Internal EF Core API usage -- EF demands usable constructors, even if we would use an interceptor that prevents their usage entirely
            var underlyingEntityType = entityType as EntityType ?? throw new NotImplementedException("Internal changes to the EF Core API have broken this code. Are public methods now available to configure instantiation?");
            underlyingEntityType.ConstructorBinding = new UninitializedBinding(entityType.ClrType);
#pragma warning restore EF1001 // Internal EF Core API usage
        }
    }

    /// <summary>
    /// An <see cref="InstantiationBinding"/> that produces uninitialized objects.
    /// </summary>
    private sealed class UninitializedBinding(
        Type runtimeType)
        : InstantiationBinding([])
    {
        private static readonly MethodInfo GetUninitializedObjectMethod = typeof(RuntimeHelpers).GetMethod(nameof(RuntimeHelpers.GetUninitializedObject))!;

        public override Type RuntimeType { get; } = runtimeType;

        public override Expression CreateConstructorExpression(ParameterBindingInfo bindingInfo)
        {
            var result = Expression.Convert(
                Expression.Call(method: GetUninitializedObjectMethod, arguments: Expression.Constant(this.RuntimeType)),
                this.RuntimeType);
            return result;
        }

        public override InstantiationBinding With(IReadOnlyList<ParameterBinding> parameterBindings)
        {
            return this;
        }
    }
}