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.8k stars 3.2k forks source link

Throw better exception for non-ownership navigations to the owner #21021

Open RazvanB opened 4 years ago

RazvanB commented 4 years ago

TestFKSameTable.zip When trying to add a migration for attached example project, I'll get the following.

System.NullReferenceException: Object reference not set to an instance of an object.
   at Microsoft.EntityFrameworkCore.Migrations.Design.CSharpSnapshotGenerator.GenerateEntityType(String builderName, IEntityType entityType, IndentedStringBuilder stringBuilder)
   at Microsoft.EntityFrameworkCore.Migrations.Design.CSharpSnapshotGenerator.GenerateOwnedType(String builderName, IForeignKey ownership, IndentedStringBuilder stringBuilder)
   at Microsoft.EntityFrameworkCore.Migrations.Design.CSharpSnapshotGenerator.GenerateOwnedTypes(String builderName, IEnumerable`1 ownerships, IndentedStringBuilder stringBuilder)
   at Microsoft.EntityFrameworkCore.Migrations.Design.CSharpSnapshotGenerator.GenerateRelationships(String builderName, IEntityType entityType, IndentedStringBuilder stringBuilder)
   at Microsoft.EntityFrameworkCore.Migrations.Design.CSharpSnapshotGenerator.GenerateEntityTypeRelationships(String builderName, IEntityType entityType, IndentedStringBuilder stringBuilder)
   at Microsoft.EntityFrameworkCore.Migrations.Design.CSharpSnapshotGenerator.GenerateEntityTypes(String builderName, IReadOnlyList`1 entityTypes, IndentedStringBuilder stringBuilder)
   at Microsoft.EntityFrameworkCore.Migrations.Design.CSharpSnapshotGenerator.Generate(String builderName, IModel model, IndentedStringBuilder stringBuilder)
   at Microsoft.EntityFrameworkCore.Migrations.Design.CSharpMigrationsGenerator.GenerateMetadata(String migrationNamespace, Type contextType, String migrationName, String migrationId, IModel targetModel)
   at Microsoft.EntityFrameworkCore.Migrations.Design.MigrationsScaffolder.ScaffoldMigration(String migrationName, String rootNamespace, String subNamespace, String language)
   at Microsoft.EntityFrameworkCore.Design.Internal.MigrationsOperations.AddMigration(String name, String outputDir, String contextType)
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.AddMigrationImpl(String name, String outputDir, String contextType)
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.AddMigration.<>c__DisplayClass0_0.<.ctor>b__0()
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.OperationBase.<>c__DisplayClass3_0`1.<Execute>b__0()
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.OperationBase.Execute(Action action)
Object reference not set to an instance of an object.

Steps to reproduce

I made the following structure as an example of what I want to accomplish.

public class LinkedCondition
{
    protected  LinkedCondition() {}

    public LinkedCondition(Guid? linkedCondition, LogicalOperator? @operator)
    {
        Operator = @operator;
        ConditionId = linkedCondition.HasValue ? new ConditionId(linkedCondition.Value) : null;
    }
    public LogicalOperator? Operator { get;  }
    public ConditionId? ConditionId { get; }
    public Condition Condition { get; }
}

public class Condition
{
    protected  Condition() {}

    public Condition(string name, string expression, Guid? conditionId, LogicalOperator @operator)
    {
        Id = new ConditionId(Guid.NewGuid());
        Expression = expression;
        LinkedCondition = new LinkedCondition(conditionId, @operator);
    }

    public ConditionId Id { get; }
    public string Expression { get; }
    public LinkedCondition LinkedCondition { get; }
}

public class ConditionId
{
    public ConditionId(Guid id)
    {
        Value = id;
    }

    public Guid Value { get; }
}

And this is my DbContext configuration

public class ConditionContext : DbContext
{
    public DbSet<Condition> Conditions { get; set; }

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        var builder = modelBuilder.Entity<Condition>();

        builder.ToTable("Conditions");
        builder
            .Property(x => x.Id)
            .HasConversion(new ValueConverter<ConditionId, Guid>(y => y.Value, 
                x => new ConditionId(x)))
            .ValueGeneratedNever();

        builder.Property(x => x.Expression);

        var ownedCondition = builder
            .OwnsOne(x => x.LinkedCondition);

        ownedCondition
            .Property(x => x.ConditionId)
            .IsRequired();

        ownedCondition
            .Property(x => x.Operator)
            .HasColumnName("Operator");

        ownedCondition
            .HasOne(x => x.Condition)
            .WithMany()
            .HasForeignKey(x => x.ConditionId);
    }
} 

I have done some debugging and I found that the Null Reference Exception happens at line var ownerNavigation = ownership?.PrincipalToDependent.Name; from method

Microsoft.EntityFrameworkCore.Migrations.DesignCSharpSnapshotGenerator. GenerateEntityType(
            [NotNull] string builderName,
            [NotNull] IEntityType entityType,
            [NotNull] IndentedStringBuilder stringBuilder)

This is because PrincipalToDependent is null.

If I replace that line with the following code the migration is completed because the navigation is found in DependentToPrincipal property

string ownerNavigation = null;

if (ownership?.DependentToPrincipal != null)
{
    ownerNavigation = ownership?.DependentToPrincipal?.Name;
} else if (ownership?.PrincipalToDependent != null)
{
    ownerNavigation = ownership?.PrincipalToDependent.Name;
}

Further technical details

EF Core version: 3.1 Database provider: Microsoft.EntityFrameworkCore.SqlServer Target framework: .NET Core 3.1 Operating system: Windows 10 v1909 IDE: Visual Studio 2019 16.5.4

ajcvickers commented 4 years ago

@AndriySvyryd I am able to reproduce this on the latest daily build. Minimal code and model below. I'm not entirely sure what this does:

var builder = modelBuilder.Entity<Condition>()
    .OwnsOne(x => x.LinkedCondition)
    .HasOne(x => x.Condition)
    .WithMany()
    .HasForeignKey(x => x.ConditionId);
public static class Program
{
    public static void Main()
    {
        using (var context = new SomeDbContext())
        {
            context.Database.EnsureDeleted();
            context.Database.EnsureCreated();
        }
    }
}

public class LinkedCondition
{
    public Guid? ConditionId { get; }
    public Condition Condition { get; }
}

public class Condition
{
    public Guid Id { get; set; }
    public LinkedCondition LinkedCondition { get; }
}

public class SomeDbContext : DbContext
{
    private static readonly ILoggerFactory
        Logger = LoggerFactory.Create(x => x.AddConsole()); //.SetMinimumLevel(LogLevel.Debug));

    public DbSet<Condition> Conditions { get; set; }

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        var builder = modelBuilder.Entity<Condition>()
            .OwnsOne(x => x.LinkedCondition)
            .HasOne(x => x.Condition)
            .WithMany()
            .HasForeignKey(x => x.ConditionId);
    }    

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        => optionsBuilder
            .UseLoggerFactory(Logger)
            .EnableSensitiveDataLogging()
            .UseSqlServer(Your.SqlServerConnectionString);
}
Model: 
  EntityType: Condition
    Properties: 
      Id (Guid) Required PK AfterSave:Throw ValueGenerated.OnAdd
    Keys: 
      Id PK
  EntityType: LinkedCondition
    Properties: 
      ConditionId (Nullable<Guid>) Required PK FK AfterSave:Throw
      Id (no field, int) Shadow Required PK AfterSave:Throw ValueGenerated.OnAdd
    Navigations: 
      Condition (Condition) ToPrincipal Condition
    Keys: 
      ConditionId, Id PK
    Foreign keys: 
      LinkedCondition {'ConditionId'} -> Condition {'Id'} Ownership ToPrincipal: Condition Cascade
smitpatel commented 4 years ago

My observations,

RazvanB commented 4 years ago

To explain further what I want to obtain with this example.

What I want to obtain is that I have, in terms of DDD, a value object LinkedCondition which contain another Condition Id.

And I want to add a FK to the same table for it.

The PK ConditionId is converted to a primitive wrapper to become also a value object.

ajcvickers commented 4 years ago

@RazvanB I don't think EF Core can currently support the model you want to build, and I'm not entirely sure what it means in DDD to have a value object with a navigation property. At least for the moment, we're putting this on the backlog to throw a better exception when this kind of mapping is attempted.