Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Query: ThenInclude throw ArgumentNullException when entity implements the interface #3409

Closed
Boglen opened this issue Oct 12, 2015 · 5 comments
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Milestone

Comments

@Boglen
Copy link

Boglen commented Oct 12, 2015

public interface ITestEntity1
{
    int Id { get; set; }

    ICollection<ITestEntity2> EntityChilds { get; set; }
}

public interface ITestEntity2
{
    int Id { get; set; }

    ICollection<ITestEntity2> Childs { get; set; }

    int? TestEntity1Id { get; set; }
    ITestEntity1 ParentRef { get; set; }
    int? TestEntity2Id { get; set; }
    ITestEntity2 Parent { get; set; }
}

public class TestEntity11 : ITestEntity1
{
    public int Id { get; set; }

    public virtual ICollection<ITestEntity2> EntityChilds { get; set; }
}

public class TestEntity22 : ITestEntity2
{
    public int Id { get; set; }

    public virtual ICollection<ITestEntity2> Childs { get; set; }

    public int? TestEntity1Id { get; set; }
    public ITestEntity1 ParentRef { get; set; }
    public int? TestEntity2Id { get; set; }
    public ITestEntity2 Parent { get; set; }
}

public class TestContext22 : DbContext
{
    public DbSet<TestEntity11> Entity1 { get; set; }
    public DbSet<TestEntity22> Entity2 { get; set; }

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        base.OnModelCreating(modelBuilder);
        modelBuilder.HasAnnotation("SqlServer:ValueGenerationStrategy", SqlServerValueGenerationStrategy.IdentityColumn);

        modelBuilder.Entity<TestEntity11>(b => {
            b.HasKey(k => k.Id);
            b.HasMany(p => (ICollection<TestEntity22>)p.EntityChilds).WithOne(o => (TestEntity11)o.ParentRef).HasForeignKey(k => k.TestEntity1Id).IsRequired(false);
        });

        modelBuilder.Entity<TestEntity22>(b => {
            b.HasKey(k => k.Id);
            b.HasMany(p => (ICollection<TestEntity22>)p.Childs).WithOne(o => (TestEntity22)o.Parent).HasForeignKey(k => k.TestEntity2Id).IsRequired(false);
        });
    }

}

        using (var context = new TestContext22())
        {
            context.Database.EnsureDeleted();
            context.Database.EnsureCreated();

            var ent1 = context.Add(new TestEntity11()).Entity;
            context.SaveChanges();

            var ent21 = context.Add(new TestEntity22()).Entity;
            var ent22 = context.Add(new TestEntity22()).Entity;
            var ent23 = context.Add(new TestEntity22()).Entity;

            context.SaveChanges();

            ent21.ParentRef = ent1;
            ent21.Childs = new List<ITestEntity2>() { ent22, ent23 };


            context.SaveChanges();


            //Throw exception
            var getIt = context.Entity1.Include(p => p.EntityChilds).ThenInclude(i => i.Childs).ToList();
        }
@rowanmiller
Copy link
Contributor

This is because mapping to interfaces is not supported. This means public virtual ICollection<ITestEntity2> EntityChilds { get; set; } is not a valid navigation property and so it is not included in the model - therefore you get an exception when you try to include it.

Enabling this scenario with interfaces is already tracked by #757

@divega
Copy link
Contributor

divega commented Oct 14, 2015

I may be missing something but it seems that this could be just a bug in ThenInclude(). The fact that b.HasMany(p => (IEnumerable<TestEntity22>)p.Childs) or b.HasMany(p => (ICollection<TestEntity22>)p.Childs) works for a Childs navigation property that is of type ICollection<ITestEntity2> seems a bit quirky (but fortunate) and is just a consequence of the fact that compiler doesn't validate the cast (at runtime the cast would not work). But other than that (and other than the failing cases):

  1. The model is built and seems to be what the user expected.
  2. The collection navigation properties become of type HashSet<ITestEntity#> at runtime but those can hold instances of the actual entity types.
  3. The data seems to be saved to the database correctly.
  4. The following scenarios all seem to work and cause the entities and nav props to be materialized/fixed up correctly:
using (var context = new TestContext22())
{
    context.Entity1.Load();
    context.Entity2.Load();
}
using (var context = new TestContext22())
{
    context.Entity1.Include(p => p.EntityChilds).Load();
}
using (var context = new TestContext22())
{
    context.Entity2.Include(p => p.Parent).Load();
}
using (var context = new TestContext22())
{
    context.Entity2.Include(p => p.ParentRef).Load();
}
using (var context = new TestContext22())
{
    context.Entity2.Include(p => p.Childs).Load();
}
using (var context = new TestContext22())
{
    context.Entity2.Include(p => p.Parent).Include(p => p.ParentRef).Load();
}

These two cases fail with ArgumentException:

using (var context = new TestContext22())
{
    context.Entity1.Include(p => p.EntityChilds).ThenInclude(i => i.Childs).Load();
}
using (var context = new TestContext22())
{
    context.Entity2.Include(p => p.Parent).ThenInclude(i => i.ParentRef).Load();
}

(Note: I tried all this with beta8)

@divega divega reopened this Oct 14, 2015
@Boglen
Copy link
Author

Boglen commented Oct 14, 2015

In intellisence after i expand Entity1 and Entity2 and then expand context.Entity1.EntityChilds.Childs - all is good. Simple include with public virtual ICollection EntityChilds { get; set; } work fine with interfaces.

@mikary
Copy link
Contributor

mikary commented Apr 18, 2016

Ran the original reproduction and the additional cases mentioned by @divega and they are all executing without throwing. There have been several changes to Include since this issue was filed, and it is likely one of these changes also resolved this issue.

@mikary mikary closed this as completed Apr 18, 2016
@divega
Copy link
Contributor

divega commented Apr 18, 2016

@mikary do we have any test that looks like this scenario? we shouldn't regress it...

@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Oct 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Projects
None yet
Development

No branches or pull requests

6 participants