Adding an Entity Graph with sub-n-level entities with the same Id results in error - Breaking Change

[jprsilva]

EFCore 3.1.1 - breaking change
Adding a disconnected entity graph to EF Context, if there is an entity of the same type with the same Id this results in an error.

In the previous version 2.2 and in the EF6 this was working fine.

Steps to reproduce

    public class SampleContext : DbContext
    {
        public DbSet<Book> Books { get; set; }
        public DbSet<Author> Authors { get; set; }
        public DbSet<Shelf> Shelfs { get; set; }
        public DbSet<ShelfBooks> ShelfBooks { get; set; }

        public SampleContext()
            :base()
        {
            //  Detaching an entity results in related entities being deleted
            //  https://github.com/dotnet/efcore/issues/18982
            base.ChangeTracker.CascadeDeleteTiming = CascadeTiming.OnSaveChanges;
            base.ChangeTracker.DeleteOrphansTiming = CascadeTiming.OnSaveChanges;
        }
    }
    public class Author
    {
        public int AuthorId { get; set; }
        public string FirstName { get; set; }
        public string LastName { get; set; }
        public ICollection<Book> Books { get; set; }
    }
    public class Book
    {
        public int BookId { get; set; }
        public string Title { get; set; }
        public Author Author { get; set; }
        public int AuthorId { get; set; }
    }
    public class Shelf
    {
        public int ShelfId { get; set; }
        public string Description { get; set; }
        public ICollection<ShelfBooks> ShelfBooks { get; set; }

    }
    public class ShelfBooks
    {
        public int ShelfBookId { get; set; }
        public int ShelfId { get; set; }
        public Book Shelf { get; set; }
        public int BookId { get; set; }
        public Book Book { get; set; }
    }
    
    public class Test
    {
        public void AddToEFC()
        {
            var shelf = new Shelf();
            shelf.ShelfId = 0;
            shelf.Description = "New Shelf";

            shelf.ShelfBooks = new List<ShelfBooks>();
            var sb1 = new ShelfBooks
            {
                ShelfId = 0,
                Book = new Book
                {
                    BookId = 100,
                    Author = new Author
                    {
                        AuthorId = 2,
                        FirstName = "Existing Author",
                        LastName = "2"
                    },
                    Title = "Existing Book1"
                }
            };

            shelf.ShelfBooks.Add(sb1);

            var sb2 = new ShelfBooks
            {
                ShelfId = 0,
                Book = new Book
                {
                    BookId = 200,
                    Author = new Author
                    {
                        AuthorId = 2,
                        FirstName = "Existing Author",
                        LastName = "2"
                    },
                    Title = "Existing Book2"
                }
            };

            shelf.ShelfBooks.Add(sb2);

            var ctx = new SampleContext();
            ctx.Shelfs.Add(shelf);      // throws error
        }
    }

Adding the shelf entity to the EFCore context results in the error:
Message:
The instance of entity type 'Author' cannot be tracked because another instance with the key value '{AuthorId: 2}' is already being tracked. When attaching existing entities, ensure that only one entity instance with a given key value is attached.

StackTrace:

at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.IdentityMap`1.ThrowIdentityConflict(InternalEntityEntry entry)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.IdentityMap`1.Add(TKey key, InternalEntityEntry entry, Boolean updateDuplicate)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.StartTracking(InternalEntityEntry entry)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalEntityEntry.SetEntityState(EntityState oldState, EntityState newState, Boolean acceptChanges, Boolean modifyProperties)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityGraphAttacher.PaintAction(EntityEntryGraphNode`1 node)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityEntryGraphIterator.TraverseGraph[TState](EntityEntryGraphNode`1 node, Func`2 handleNode)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityEntryGraphIterator.TraverseGraph[TState](EntityEntryGraphNode`1 node, Func`2 handleNode)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityEntryGraphIterator.TraverseGraph[TState](EntityEntryGraphNode`1 node, Func`2 handleNode)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityEntryGraphIterator.TraverseGraph[TState](EntityEntryGraphNode`1 node, Func`2 handleNode)
   at Microsoft.EntityFrameworkCore.DbContext.SetEntityState(InternalEntityEntry entry, EntityState entityState)
   at Microsoft.EntityFrameworkCore.DbContext.SetEntityState[TEntity](TEntity entity, EntityState entityState)

Further technical details

EF Core version: 3.1.1
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: .NET 4.6.1
Operating system: Win 10 Pro 1909
IDE: Visual Studio 2019 16.4.5

[ajcvickers]

@jprsilva I am not able to reproduce this. Please also post the code from OnModelCreating that configures this model.

[jprsilva]

I will do it.

[jprsilva]

@ajcvickers can you use my test project?
Here it is:
https://github.com/jprsilva/EFCore311_Issue19984

Code OnModelCreating / OnConfiguring:

protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            //  Connection
            optionsBuilder.UseSqlServer("server=.;database=BooksDb;trusted_connection=true;");

            //  Logging
            optionsBuilder.UseLoggerFactory(LoggingFactory.LoggerFactory)
                .EnableSensitiveDataLogging();
        }

Error:

[jprsilva]

@ajcvickers were you able to reproduce the error?

[ajcvickers]

@jprsilva Yes. The code is attempting to track two entities with the same key value. I ran this code on 2.2 and got the same behavior. I suspect that your repro code is not setting up the same scenario as is failing in your real application.

Without being able to reproduce the issue I can't say for sure, but I have a hunch it is related to this: https://docs.microsoft.com/en-us/ef/core/what-is-new/ef-core-3.0/breaking-changes#dc

[jprsilva]

@ajcvickers Thanks for your feedback.
I made another test project with EFCore 2.2.6 and as you can see it works fine without any error with the method Add.
https://github.com/jprsilva/EFCore311_Issue19984/tree/master/EFCore226

Code with EFCore 2.2.6

using (var ctx = new EFC.BooksDbContext())
{
	ctx.Shelfs.Add(shelf);      // Does not throws any error - EFCore 3.1.1. throws error

	var addedEntities = ctx.ChangeTracker.Entries()
		.Where(x => x.State == Microsoft.EntityFrameworkCore.EntityState.Added)
		.Count();
	var updatedEntities = ctx.ChangeTracker.Entries()
		.Where(x => x.State == Microsoft.EntityFrameworkCore.EntityState.Modified)
		.Count();
	var unchagedEntities = ctx.ChangeTracker.Entries()
		.Where(x => x.State == Microsoft.EntityFrameworkCore.EntityState.Unchanged)
		.Count();
	var detachedEntities = ctx.ChangeTracker.Entries()
		.Where(x => x.State == Microsoft.EntityFrameworkCore.EntityState.Detached)
		.Count();

	Console.WriteLine($"Added entities: {addedEntities}");
	Console.WriteLine($"Updated entities: {updatedEntities}");
	Console.WriteLine($"Unchaged entities: {unchagedEntities}");
	Console.WriteLine($"Detached entities: {detachedEntities}");
}

Yes, I believe this is related to that breaking change.
So my question is, how can I disable that?
(And return to the old behavior - so I can track this kind of things myself).

[ajcvickers]

@jprsilva BookId is configured to use generated key values:

[DatabaseGenerated(DatabaseGeneratedOption.Identity)]
public int BookId { get; set; }

This means that when the value is set EF will treat the entity as an existing entry with the given key value. For example, here:

Book = new Book
{
    BookId = 200,

If you want to specify the key value for new entity instances, then configure EF for that:

[DatabaseGenerated(DatabaseGeneratedOption.None)]
public int BookId { get; set; }

If you want the database to generated key values, then don't explicitly set the BlogId property.

[jprsilva]

@ajcvickers the error is not related to the BookId.

Did you run my tests?
Did you saw the different result between EFCore 2.2 and 3.1?

The error is related when I have two instances of the same type with the same Id in a graph of entities, when I Add that graph to the context in EFCore this results in a error.

As you can see in EFCore 2.2.6 (and also EF6) this kind of error doesn’t happen.
Using the Add to context, all the entities in the added graph should assume the added state - and if an entity with the Id already exists in the context it shouldn’t throw any error - like in EFCore 2.2.6 and EF6.
At least provide some configuration for this to be possible.
Until EFCore 3 the only way to attach a full graph without any error was with Add method.
Right now I don’t have a way to do that.

Do you understand now what I am trying explain you?

I guess if you run both codes you can see the problem / difference.

[ajcvickers]

@jprsilva Do you want key values for BookId to be generated by the database?

[jprsilva]

@jprsilva Do you want key values for BookId to be generated by the database?

Yes.

[ajcvickers]

@jprsilva Then don't set BookId. Leave it set to the CLR default--that is, zero.

[jprsilva]

@jprsilva Then don't set BookId. Leave it set to the CLR default--that is, zero.

@ajcvickers if the book is an existing one I need to set it’s Id (did you saw the title “Existing Book”?).
But do you understand that the BookId is not the problem?
Working with a disconnected graph, if the book is an existing one, I need to set is Id - that’s the case in the example.
But the entity root (Shelf) is new - so I am not setting is Id, and calling the Add to set the full graph as added state.
After that I have a helper class (my own rules) that will correct the states of the entities of the graph.

[jprsilva]

Let me make a recap.

Assume I have a disconnected graph of entities that I want to attach to EF Context.
The root entity is a new entity.
Some other entities in the graph may be also new or existing entities.
Now assume that in the graph we have two instances of the same entity type with the same Id (key) - existing entities.

In the previous versions the Add method didn’t throw any error when adding a full disconnected graph, because an untracked entity found by DetectChanges would be tracked in the Added state.

Now with EFCore 3, with the breaking change: https://docs.microsoft.com/en-us/ef/core/what-is-new/ef-core-3.0/breaking-changes#dc it throws an error because it is guessing the track state of each entity and it doesn’t allow to have two entities of the same type with the same Id with the state Modified - of course.

The reported ideia is good.
But for the exposed example that I gave it doesn’t work and I need more control.
For me basically is the return to the old behavior: Before EF Core 3.0, an untracked entity found by DetectChanges would be tracked in the Added state.

I am only focusing the Add method: the attach / tracking of the entities.
I am not focusing the SaveChanges.
Adding a disconnected graph all the untracked entities should be tracked as added.
Knowing by “hand” that some entities are not new because their Id (key) is database generated and they have a value bigger than zero (CLR default - zero - means new).

So my desire is to know how to disable this behavior in EFCore 3.

I want to be able to add a disconnected graph and all the untracked entities beeing tracked as added - despite their key values and the database generate key flag.

After that I run my own helper class to update the track state of each entity in that graph.
And only after this I am able to call the SaveChanges.

Hope you can understand and help me.

[jprsilva]

I think this is similar to what you said here: #20116 (comment)
Like, @vflame said: I think EF Core should be able to detect that the same primary key should map to the same entity.

But like I already said, for me, and for the moment, if I could disable this new behavior in EF Core 3 and return to the old behavior, it would fix my problems.
https://docs.microsoft.com/en-us/ef/core/what-is-new/ef-core-3.0/breaking-changes#dc

Thanks

[ajcvickers]

@jprsilva I went back and re-investigated from the beginning. It turns out that in 2.2 your code was making use of this bug: #18007. So EF was, incorrectly, silently discarding the duplicate instances of the Author entity. Unfortunately, this behavior, though a bug, is what you were using to get the behavior you wanted.

EF Core does not, in general, allow multiple instances with the same key to be tracked, regardless of the state of the entity. (Deleted entities can have special behavior, but that's not relevant here.) The approach of going through an invalid graph state in order to then fix it up later is not recommended, even though the limitations of EF6 made it a common approach there. (Note that EF6 still required identity resolution to a single instance.)

To fix this:

• Do entity resolution before attaching the graph--you need to decide which of the multiple instances EF should use. See Provide a hook for identity resolution #20124 and Entity already tracked exception for entity with multiple references to same navigation property #20116 (comment)
• If the mechanism of using the key value to determine new/existing changes is not sufficient, then consider using the context.ChangeTracker.TrackGraph API. Note, however, that it still won't allow tracking two instances with the same ID.

[jprsilva]

Makes sense.
I thought that discarding the duplicate instances was an expected behavior, using the Add method to context.
I am going to see how manage this situation.
I will follow the evolution of the “hook for identity resolution“.

Thanks very much for your time and explanation.