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.63k stars 3.15k forks source link

Prevent mapping intrinsic types as complex types #32695

Open lsymds opened 8 months ago

lsymds commented 8 months ago

Overview

Whilst messing around with complex types in an attempt to persist a DateTimeOffset as two separate columns (one containing the ticks, and another containing the offset ticks), I received an error telling me the following:

System.ArgumentException: The specified type 'System.DateTimeOffset' must be a non-interface type with a public constructor to be used as a complex type.

DateTimeOffset is not an interface type AND has a public constructor available to be used, so I assumed it was something else.

After delving into the ComplexType constructor, and subsequently the SharedTypeExtensions.IsValidComplexType extension method, I noticed that there is a check that identifies whether the provided type is a scalar type (which DateTimeOffset is). The thrown exception's message does not indicate this, and, although an unlikely occurrence, probably should?

Repro steps

Steps

  1. Attempt to define a DateTimeOffset (or another scalar type) as a complex property on a model.
  2. Migrate the database.
  3. Boom.

Code

using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.DependencyInjection;

var serviceCollection = new ServiceCollection();
serviceCollection.AddDbContext<DatabaseContext>(c => c.UseSqlite("Data Source=:memory:"));
var serviceProvider = serviceCollection.BuildServiceProvider();

var dbContext = serviceProvider.GetService<DatabaseContext>();
dbContext.Database.Migrate();

public class Model
{
    public DateTimeOffset Date { get; set; }
}

public class DatabaseContext : DbContext
{
    public DbSet<Model> Models { get; set; }

    public DatabaseContext(DbContextOptions<DatabaseContext> options) : base(options)
    {
    }

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder
            .Entity<Model>()
            .ComplexProperty(p => p.Date);
    }
}

System Information

EF Core version: 8.0.0 Database provider: (e.g. Microsoft.EntityFrameworkCore.SqlServer) Microsoft.EntityFrameworkCore.Sqlite Target framework: (e.g. .NET 6.0) .NET 8.0 Operating system: Windows 11 IDE: (e.g. Visual Studio 2022 17.4) Jetbrains Rider 2023.3

ajcvickers commented 7 months ago

Notes for triage: I spent a bit more (too much) time investigating this further. With a few tweaks to allow constructor configuration, the following works:

modelBuilder.Entity<Foo>().ComplexProperty(
    e => e.Date, b =>
    {
        var timeProperty = b.Property(e => e.UtcDateTime).HasField("_dateTime").Metadata;
        var offsetProperty = b.Property<short>("_offsetMinutes").Metadata;

        var complexType = (ComplexType)b.Metadata.ComplexType;
        complexType.ConstructorBinding = new ConstructorBinding(
            typeof(DateTimeOffset)
                .GetConstructors(BindingFlags.Instance | BindingFlags.NonPublic)
                .Single(
                    c =>
                    {
                        var parameters = c.GetParameters();
                        return parameters.Length == 2
                            && parameters[0].Name == "validOffsetMinutes"
                            && parameters[1].Name == "validDateTime";
                    }),
            new[]
            {
                new PropertyParameterBinding((IProperty)offsetProperty),
                new PropertyParameterBinding((IProperty)timeProperty)
            });
    });

Whether or not this is a good idea, since:

Still, it's cool we can do this. :-)

ajcvickers commented 7 months ago

Full code here: https://github.com/dotnet/efcore/tree/240118_NotSoSimpleAfterAll