Attaching an entity with a mix of existing and new entities in its graph (Entity Framework Core 1.1.0)

Question

I have encountered an issue when attaching entities holding reference properties to existing entities (I call existing entity an entity that already exists in the database, and has its PK properly set).

The issue is when using Entity Framework Core 1.1.0. This is something that was working perfectly with Entity Framework 7 (the initial name of Entity Framework Core).

I haven't tried it neither with EF6 nor with EF Core 1.0.0.

I wonder if this is a regression, or a change of behaviour made on purpose.

The model

The test model consists in Place, Person, and a many-to-many relationship between Place and Person, through a joining entity named PlacePerson.

public abstract class BaseEntity
{
    public int Id { get; set; }
    public string Name { get; set; }
}

public class Person : BaseEntity
{
    public int? StatusId { get; set; }
    public Status Status { get; set; }
    public List<PersonPlace> PersonPlaceCollection { get; set; } = new List<PersonPlace>();
}

public class Place : BaseEntity
{
    public List<PersonPlace> PersonPlaceCollection { get; set; } = new List<PersonPlace>();
}

public class PersonPlace : BaseEntity
{
    public int? PersonId { get; set; }
    public Person Person { get; set; }
    public int? PlaceId { get; set; }
    public Place Place { get; set; }
}

The database context

All relationships are explicitely defined (without redundancy).

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

        // PersonPlace
        builder.Entity<PersonPlace>()
            .HasAlternateKey(o => new { o.PersonId, o.PlaceId });
        builder.Entity<PersonPlace>()
            .HasOne(pl => pl.Person)
            .WithMany(p => p.PersonPlaceCollection)
            .HasForeignKey(p => p.PersonId);
        builder.Entity<PersonPlace>()
            .HasOne(p => p.Place)
            .WithMany(pl => pl.PersonPlaceCollection)
            .HasForeignKey(p => p.PlaceId);
    }

All concrete entities are also exposed in this model:

public DbSet<Person> PersonCollection { get; set; } 
public DbSet<Place> PlaceCollection { get; set; }
public DbSet<PersonPlace> PersonPlaceCollection { get; set; }

Factoring data access

I am using a Repository-style base class to factor all data-access related code.

public class DbRepository<T> where T : BaseEntity
{
    protected readonly MyContext _context;
    protected DbRepository(MyContext context) { _context = context; }

    // AsNoTracking provides detached entities
    public virtual T FindByNameAsNoTracking(string name) => 
        _context.Set<T>()
            .AsNoTracking()
            .FirstOrDefault(e => e.Name == name);

    // New entities should be inserted
    public void Insert(T entity) => _context.Add(entity);
    // Existing (PK > 0) entities should be updated
    public void Update(T entity) => _context.Update(entity);
    // Commiting
    public void SaveChanges() => _context.SaveChanges();
}

Steps to reproduce the exception

Create one person and save it. Create one Place and save it.

// Repo
var context = new MyContext()
var personRepo = new DbRepository<Person>(context);
var placeRepo = new DbRepository<Place>(context);

// Person
var jonSnow = new Person() { Name = "Jon SNOW" };
personRepo.Add(jonSnow);
personRepo.SaveChanges();

// Place
var castleblackPlace = new Place() { Name = "Castleblack" };
placeRepo.Add(castleblackPlace);
placeRepo.SaveChanges();

Both the person and the place are in the database, and thus have a primary key defined. PK are generated as identity columns by SQL Server.

Reload the person and the place, as detached entities (the fact they are detached is used to mock a scenario of http posted entities through a web API, e.g. with angularJS on client side).

// detached entities
var jonSnow = personRepo.FindByNameAsNoTracking("Jon SNOW");
var castleblackPlace = placeRepo.FindByNameAsNoTracking("Castleblack");

Add the person to the place and save this:

castleblackPlace.PersonPlaceCollection.Add(
    new PersonPlace()  { Person = jonSnow }
);
placeRepo.Update(castleblackPlace);
placeRepo.SaveChanges();

On SaveChanges an exception is thrown, because EF Core 1.1.0 tries to INSERT the existing person instead of doing an UPDATE (though its primary key value is set).

Exception details

Microsoft.EntityFrameworkCore.DbUpdateException: An error occurred while updating the entries. See the inner exception for details. ---> System.Data.SqlClient.SqlException: Cannot insert explicit value for identity column in table 'Person' when IDENTITY_INSERT is set to OFF.

Previous versions

This code would work perfectly (though not necessarily optimized) with the alpha version of EF Core (named EF7) and the DNX CLI.

Workaround

Iterate over the root entity graph and properly set the Entity states:

_context.ChangeTracker.TrackGraph(entity, node =>
    {
        var entry = node.Entry;
        var childEntity = (BaseEntity)entry.Entity;
        entry.State = childEntity.Id <= 0? EntityState.Added : EntityState.Modified;
    });

What's the question at last ???

Why do we have to manually track the entity states, whereas previous versions of EF would totally deal with it, even when reattaching detached entities ?

Full reproduction source (EFCore 1.1.0 - not working)

Full reproduction source (the workaround described above is included but its call is commented. Uncommenting it will make this source work).

using System.Collections.Generic;
using System.ComponentModel.DataAnnotations.Schema;
using System.Linq;
using Microsoft.EntityFrameworkCore;

namespace EF110CoreTest
{
    public class Program
    {
        public static void Main(string[] args)
        {
            // One scope for initial data
            using (var context = new MyContext())
            {
                // Repo
                var personRepo = new DbRepository<Person>(context);
                var placeRepo = new DbRepository<Place>(context);

                // Database
                context.Database.EnsureDeleted();
                context.Database.EnsureCreated();


                /***********************************************************************/

                // Step 1 : Create a person
                var jonSnow = new Person() { Name = "Jon SNOW" };
                personRepo.InsertOrUpdate(jonSnow);
                personRepo.SaveChanges();

                /***********************************************************************/

                // Step 2 : Create a place
                var castleblackPlace = new Place() { Name = "Castleblack" };
                placeRepo.InsertOrUpdate(castleblackPlace);
                placeRepo.SaveChanges();

                /***********************************************************************/
            }

            // Another scope to put one people in one place
            using (var context = new MyContext())
            {
                // Repo
                var personRepo = new DbRepository<Person>(context);
                var placeRepo = new DbRepository<Place>(context);

                // entities
                var jonSnow = personRepo.FindByNameAsNoTracking("Jon SNOW");
                var castleblackPlace = placeRepo.FindByNameAsNoTracking("Castleblack");

                // Step 3 : add person to this place
                castleblackPlace.AddPerson(jonSnow);
                placeRepo.InsertOrUpdate(castleblackPlace);
                placeRepo.SaveChanges();
            }

        }
    }

    public class DbRepository<T> where T : BaseEntity
    {
        public readonly MyContext _context;
        public DbRepository(MyContext context) { _context = context; }

        public virtual T FindByNameAsNoTracking(string name) => _context.Set<T>().AsNoTracking().FirstOrDefault(e => e.Name == name);

        public void InsertOrUpdate(T entity)
        {
            if (entity.IsNew) Insert(entity); else Update(entity);
        }

        public void Insert(T entity)
        {
            // uncomment to enable workaround
            //ApplyStates(entity);
            _context.Add(entity);
        }

        public void Update(T entity)
        {
            // uncomment to enable workaround
            //ApplyStates(entity);
            _context.Update(entity);
        }

        public void Delete(T entity)
        {
            _context.Remove(entity);
        }

        private void ApplyStates(T entity)
        {
            _context.ChangeTracker.TrackGraph(entity, node =>
            {
                var entry = node.Entry;
                var childEntity = (BaseEntity)entry.Entity;
                entry.State = childEntity.IsNew ? EntityState.Added : EntityState.Modified;
            });
        }

        public void SaveChanges() => _context.SaveChanges();
    }

    #region Models
    public abstract class BaseEntity
    {
        public int Id { get; set; }
        public string Name { get; set; }
        [NotMapped]
        public bool IsNew => Id <= 0;
        public override string ToString() => $"Id={Id} | Name={Name} | Type={GetType()}";
    }

    public class Person : BaseEntity
    {
        public List<PersonPlace> PersonPlaceCollection { get; set; } = new List<PersonPlace>();
        public void AddPlace(Place place) => PersonPlaceCollection.Add(new PersonPlace { Place = place });
    }

    public class Place : BaseEntity
    {
        public List<PersonPlace> PersonPlaceCollection { get; set; } = new List<PersonPlace>();
        public void AddPerson(Person person) => PersonPlaceCollection.Add(new PersonPlace { Person = person, PersonId = person?.Id, PlaceId = 0});
    }

    public class PersonPlace : BaseEntity
    {
        public int? PersonId { get; set; }
        public Person Person { get; set; }
        public int? PlaceId { get; set; }
        public Place Place { get; set; }
    }

    #endregion

    #region Context
    public class MyContext : DbContext
    {
        public DbSet<Person> PersonCollection { get; set; } 
        public DbSet<Place> PlaceCollection { get; set; }
        public DbSet<PersonPlace> PersonPlaceCollection { get; set; }

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


            // PersonPlace
            builder.Entity<PersonPlace>()
                .HasAlternateKey(o => new { o.PersonId, o.PlaceId });
            builder.Entity<PersonPlace>()
                .HasOne(pl => pl.Person)
                .WithMany(p => p.PersonPlaceCollection)
                .HasForeignKey(p => p.PersonId);
            builder.Entity<PersonPlace>()
                .HasOne(p => p.Place)
                .WithMany(pl => pl.PersonPlaceCollection)
                .HasForeignKey(p => p.PlaceId);
        }

        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            optionsBuilder.UseSqlServer(@"Server=(localdb)\mssqllocaldb;Database=EF110CoreTest;Trusted_Connection=True;");
        }
    }
    #endregion
}

Project.json file for EFCore1.1.0 project

{
  "version": "1.0.0-*",
  "buildOptions": {
    "emitEntryPoint": true
},

  "dependencies": {
    "Microsoft.EntityFrameworkCore": "1.1.0",
    "Microsoft.EntityFrameworkCore.SqlServer": "1.1.0",
    "Microsoft.EntityFrameworkCore.Tools": "1.1.0-preview4-final" 
},

  "frameworks": {
    "net461": {}
},

  "tools": {
    "Microsoft.EntityFrameworkCore.Tools.DotNet": "1.0.0-preview3-final"
  }
}

Working source with EF7 / DNX

using System.Collections.Generic;
using Microsoft.Data.Entity;
using System.Linq;
using System.ComponentModel.DataAnnotations.Schema;

namespace EF7Test
{
    public class Program
    {
        public static void Main(string[] args)
        {
            // One scope for initial data
            using (var context = new MyContext())
            {
                // Repo
                var personRepo = new DbRepository<Person>(context);
                var placeRepo = new DbRepository<Place>(context);

                // Database
                context.Database.EnsureDeleted();
                context.Database.EnsureCreated();


                /***********************************************************************/

                // Step 1 : Create a person
                var jonSnow = new Person() { Name = "Jon SNOW" };
                personRepo.InsertOrUpdate(jonSnow);
                personRepo.SaveChanges();

                /***********************************************************************/

                // Step 2 : Create a place
                var castleblackPlace = new Place() { Name = "Castleblack" };
                placeRepo.InsertOrUpdate(castleblackPlace);
                placeRepo.SaveChanges();

                /***********************************************************************/
            }

            // Another scope to put one people in one place
            using (var context = new MyContext())
            {
                // Repo
                var personRepo = new DbRepository<Person>(context);
                var placeRepo = new DbRepository<Place>(context);

                // entities
                var jonSnow = personRepo.FindByNameAsNoTracking("Jon SNOW");
                var castleblackPlace = placeRepo.FindByNameAsNoTracking("Castleblack");

                // Step 3 : add person to this place
                castleblackPlace.AddPerson(jonSnow);
                placeRepo.InsertOrUpdate(castleblackPlace);
                placeRepo.SaveChanges();
            }

        }
    }

    public class DbRepository<T> where T : BaseEntity
    {
        public readonly MyContext _context;
        public DbRepository(MyContext context) { _context = context; }

        public virtual T FindByNameAsNoTracking(string name) => _context.Set<T>().AsNoTracking().FirstOrDefault(e => e.Name == name);

        public void InsertOrUpdate(T entity)
        {
            if (entity.IsNew) Insert(entity); else Update(entity);
        }

        public void Insert(T entity) => _context.Add(entity);
        public void Update(T entity) => _context.Update(entity);
        public void SaveChanges() => _context.SaveChanges();
    }

    #region Models
    public abstract class BaseEntity
    {
        public int Id { get; set; }
        public string Name { get; set; }
        [NotMapped]
        public bool IsNew => Id <= 0;
        public override string ToString() => $"Id={Id} | Name={Name} | Type={GetType()}";
    }

    public class Person : BaseEntity
    {
        public List<PersonPlace> PersonPlaceCollection { get; set; } = new List<PersonPlace>();
        public void AddPlace(Place place) => PersonPlaceCollection.Add(new PersonPlace { Place = place });
    }

    public class Place : BaseEntity
    {
        public List<PersonPlace> PersonPlaceCollection { get; set; } = new List<PersonPlace>();
        public void AddPerson(Person person) => PersonPlaceCollection.Add(new PersonPlace { Person = person, PersonId = person?.Id, PlaceId = 0 });
    }

    public class PersonPlace : BaseEntity
    {
        public int? PersonId { get; set; }
        public Person Person { get; set; }
        public int? PlaceId { get; set; }
        public Place Place { get; set; }
    }

    #endregion

    #region Context
    public class MyContext : DbContext
    {
        public DbSet<Person> PersonCollection { get; set; }
        public DbSet<Place> PlaceCollection { get; set; }
        public DbSet<PersonPlace> PersonPlaceCollection { get; set; }

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

            // PersonPlace
            builder.Entity<PersonPlace>()
                .HasAlternateKey(o => new { o.PersonId, o.PlaceId });
            builder.Entity<PersonPlace>()
                .HasOne(pl => pl.Person)
                .WithMany(p => p.PersonPlaceCollection)
                .HasForeignKey(p => p.PersonId);
            builder.Entity<PersonPlace>()
                .HasOne(p => p.Place)
                .WithMany(pl => pl.PersonPlaceCollection)
                .HasForeignKey(p => p.PlaceId);
        }

        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            optionsBuilder.UseSqlServer(@"Server=(localdb)\mssqllocaldb;Database=EF7Test;Trusted_Connection=True;");
        }
    }
    #endregion
}

And the corresponding project file:

{
"version": "1.0.0-*",
"buildOptions": {
    "emitEntryPoint": true
},

"dependencies": {
    "EntityFramework.Commands": "7.0.0-rc1-*",
    "EntityFramework.Core": "7.0.0-rc1-*",
    "EntityFramework.MicrosoftSqlServer": "7.0.0-rc1-*"
},

"frameworks": {
    "dnx451": {}
},

"commands": {
    "ef": "EntityFramework.Commands"
}
}

Answer 1

After some researches, reading the comments, blog posts, and above all, the answer by an EF team member to an issue I submitted in the GitHub repo, it appears that the behaviour I noticed in my question is not a bug, but a feature of EF Core 1.0.0 and 1.1.0.

[...] in 1.1 whenever we determine that an entity should be Added because it does not have a key set, then all entities discovered as children of that entity will also be marked as Added.

(Arthur Vickers -> https://github.com/aspnet/EntityFramework/issues/7334)

So what I called 'workaround' is actually a recommended practice, as Ivan Stoev stated in his comment.

Dealing with entity states according to their primary key state

The DbContect.ChangeTracker.TrackGraph(object rootEntity, Action<EntityEntryGraphNodes> callback) method takes the root entity (the one that is posted, or added, updated, attached, whatever), and then iterates over all the discovered entities in the relationship graph of the root, and executes the callback Action.

This can be called prior to the _context.Add() or _context.Update() methods.

_context.ChangeTracker.TrackGraph(rootEntity, node => 
{ 
    node.Entry.State = n.Entry.IsKeySet ? 
        EntityState.Modified : 
        EntityState.Added; 
});

But (nothing said before 'but' actually matters!) there's something I had been missing for too long and that caused me HeadAcheExceptions:

If an entity is discovered that is already tracked by the context, that entity is not processed (and it's navigation properties are not traversed).

(source: intellisense of that method !)

So maybe it might be safe to ensure the context is free of anything before posting a disconnected entity:

public virtual void DetachAll()
{
    foreach (var entityEntry in _context.ChangeTracker.Entries().ToArray())
    {
        if (entityEntry.Entity != null)
        {
            entityEntry.State = EntityState.Detached;
        }
    }
}

Client-side state mapping

Another approach is to deal with the state on client side, post entities (therefore disconnected by design), and set their state according to a client-side state.

First, define an enum that maps client states to entity states (only the detached state is missing, because is doen't make sense):

public enum ObjectState
{
    Unchanged = 1,
    Deleted = 2,
    Modified = 3,
    Added = 4
}

Then, use the DbContect.ChangeTracker.TrackGraph(object rootEntity, Action<EntityEntryGraphNodes> callback) method to set Entity states according to client state:

_context.ChangeTracker.TrackGraph(entity, node =>
{
    var entry = node.Entry;
    // I don't like switch case blocks !
    if (childEntity.ClientState == ObjectState.Deleted) entry.State = EntityState.Deleted;
    else if (childEntity.ClientState == ObjectState.Unchanged) entry.State = EntityState.Unchanged;
    else if (childEntity.ClientState == ObjectState.Modified) entry.State = EntityState.Modified;
    else if (childEntity.ClientState == ObjectState.Added) entry.State = EntityState.Added;
});

With this approach, I use a BaseEntity abstract class, which shares the Id (PK) of my entities, and also the ClientState (of type ObjectState) (and a IsNew accessor, based on PK value)

public abstract class BaseEntity
{
    public int Id {get;set;}
    [NotMapped]
    public ObjectState ClientState { get;set; } = ObjectState.Unchanged;
    [NotMapped]
    public bool IsNew => Id <= 0;
}

Optimistic / heuristic approach

This is what I actually implemented. I have of mix of the old approach (meaning that if en entity has it's PK undefined, it must be added, and if the root has a PK, it must me updated), and the client state approach:

_context.ChangeTracker.TrackGraph(entity, node =>
{
    var entry = node.Entry;
    // cast to my own BaseEntity
    var childEntity = (BaseEntity)node.Entry.Entity;
    // If entity is new, it must be added whatever the client state
    if (childEntity.IsNew) entry.State = EntityState.Added;
    // then client state is mapped
    else if (childEntity.ClientState == ObjectState.Deleted) entry.State = EntityState.Deleted;
    else if (childEntity.ClientState == ObjectState.Unchanged) entry.State = EntityState.Unchanged;
    else if (childEntity.ClientState == ObjectState.Modified) entry.State = EntityState.Modified;
    else if (childEntity.ClientState == ObjectState.Added) entry.State = EntityState.Added;
});