Null Ref when trying to delete some entities from model

[maumar]

Repro is as follows:

public class Gear
{
    // composite key
    public string Nickname { get; set; }
    public int SquadId { get; set; }

    public string FullName { get; set; }

    public virtual Squad Squad { get; set; }

    public string LeaderNickname { get; set; }
    public int LeaderSquadId { get; set; }
}

public class Squad
{
    // non-auto generated key
    public int Id { get; set; }
    public string Name { get; set; }

    public virtual ICollection<Gear> Members { get; set; }
}

public class GearsOfWarContext : DbContext
{
    public GearsOfWarContext(DbContextOptions options)
        : base(options)
    {
    }

    public DbSet<Gear> Gears { get; set; }
    public DbSet<Squad> Squads { get; set; }

    protected override void OnModelCreating(ModelBuilder builder)
    {
        builder.Entity<Gear>().Key(g => new { g.Nickname, g.SquadId });

        builder.Entity<Squad>().Key(s => s.Id);
        builder.Entity<Squad>().OneToMany(s => s.Members, g => g.Squad).ForeignKey(g => g.SquadId);
    }
}

class Program
{
    static void Main(string[] args)
    {
        var contextOptions = new DbContextOptions();
        contextOptions.UseSqlServer(@"Data Source=.\SQLEXPRESS;Initial Catalog=DB_NULLREF;Integrated Security=True;MultipleActiveResultSets=True");

        using (var ctx = new GearsOfWarContext(contextOptions))
        {
            ctx.Database.EnsureDeleted();
            ctx.Database.EnsureCreated();

            var deltaSquad = new Squad
            {
                Id = 1,
                Name = "Delta",
                Members = new List<Gear>(),
            };

            ctx.Squads.Add(deltaSquad);
            ctx.SaveChanges();
        }

        using (var ctx = new GearsOfWarContext(contextOptions))
        {
            var squadToRemove = ctx.Squads.First();
            ctx.Squads.Remove(squadToRemove);
            ctx.SaveChanges();

        }
    }
}

[anpete]

Stack trace?

[maumar]

My bad:

Unhandled Exception: System.NullReferenceException: Object reference not set to an instance of an object. at Microsoft.Data.Entity.ChangeTracking.ChangeDetector.DetectNavigationChange (StateEntry entry, INavigation navigation) at Microsoft.Data.Entity.ChangeTracking.ChangeDetector.DetectChanges(StateEnt ry entry) at Microsoft.Data.Entity.ChangeTracking.ChangeDetector.DetectChanges(StateMan ager stateManager) at Microsoft.Data.Entity.DbContext.d__1.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNot ification(Task task) at Microsoft.Data.Entity.DbContext.SaveChanges()

[ilmax]

Maybe same as #584, Try Initialize your navigation properties inside entity ctor

[divega]

@ilmax I agree this looks like a symptom of the same issue (we are actually using #152 to track that now), but that doesn't mean that there isn't a bug in DetectNavigationChange. It should be resilient to null nav props.

[maumar]

Yes, that solves it. Before I was hitting this issue on insert, so started initializing collections for entities I was about to insert. Adding initialization to the constructor solves this for deletes as well

[divega]

Reopening based on my last comment. DetectNavigationChange shouldn't assume the nav prop is populated.

[ilmax]

@divega O.T. Note that with code as #584 + initialization of navigation properties in entity's constructor to avoid NRE, the detail entities are not inserted on SaveChanges, only the master are. Does this is currently not implemented or is an issue? If you prefer I can open a separate issue to keep track of this.

[divega]

@ilmax I am not sure I understand :smile: Anyway @ajcvickers is fixing the fact that collections isn't initialized in #637 (which should be checked in soon) but I think this is still a robustness issue in our change detection code, which assumes nav props to be initialized, that is why I want to keep it separate.

[divega]

Apparently I had misunderstood what #637 is about. It adds handling for null collections to those places in which the EF reasons about navigation properties. Most likely it fixes this bug. But it does not cover initializing the collections by default on materialization. I will file another bug for that.

[ilmax]

@divega With the code I posted in #584 essentially only the master entity is saved to db after savechanges, not the detail one. In that example code I'm newing up a master entity (Blog) and adding a new detail entity (Post). After calling savechanges only the master (Blog) in saved to database, the detail entity (Post) is not saved. If this feature should work maybe I can open a new issue for tracking it, if it's currently not implemented then please ignore those comments.

[rowanmiller]

@ilmax - This is currently by design, we don't have any logic that reasons about graphs of entities in EF7 yet (i.e. adding the Blog will only add the Blog and we don't scan navigation properties looking for other entities). We are still discussing how best to handle this, since graph behavior in earlier versions of EF has caused a lot of confusion.

Here is the work item tracking the need for discussion around this - https://github.com/aspnet/EntityFramework/issues/238.

[ilmax]

@rowanmiller Good to know! I have used EF6 in a disconnected scenario and it was a little bit painful to correctly handle graphs, so if you want any feedback/use cases I can share my thoughts and experience on this.

Max.

[rowanmiller]

@ajcvickers - is this one good to close now?