Handle entity replacement in TrackGraph with better exception/message

[camainc]

I'm writing integration tests using the In-Memory provider, and I ran into something that is causing me to scratch my head.

I have some code like this:

_dbContext.ChangeTracker.TrackGraph(
	entity, n => {
		n.Entry.State = entity.IsNew ? EntityState.Added : EntityState.Modified;
	});

This works with object graphs when going against SQL server, and each object in the graph get's its EntityState set. But when I run through it with entities fetched using the in-memory provider, it doesn't seem to traverse the graph, and only updates the base entity.

Am I doing something wrong? Or is this the expected behavior?

Thanks

Further technical details

EF Core version: 2.1.1.0
Database Provider: In-Memory
Operating system:
IDE: Visual Studio 2017 15.8.0 Preview 3.0

[ajcvickers]

@camainc Not expected. Please post a runnable project/solution or complete code listing that reproduces the behavior you are seeing.

[camainc]

This is about as simple a solution as I could put together that demonstrates the behavior. It only uses the in-memory provider, but I have run the same method (in another project) against SQL Server and it works as expected. It's only when running against in-memory that it doesn't work right.

The method that works differently is in the "TrackChangesClient" project, in the "Repository" class, in the "Update" method. The root entity gets updated when saving, but the child entity does not.

Thanks for looking at this. I need to know if I'm doing this wrong. :-)

TrackChangesTest.zip

[ajcvickers]

Note for triage: the issue is with this code, and happens with both in-memory and SqlServer provider; I don't think that it is provider-specific.

if (_dbContext.Entry(entity).State == EntityState.Detached)
{
    // get and detach existing entry so we can reattach the updated entity
    var existingEntity = _dbSet.IgnoreQueryFilters().Single(x => x.Id == entity.Id);
    _dbContext.Entry(existingEntity).State = EntityState.Detached;
}

// reattach the child entities
_dbContext.ChangeTracker.TrackGraph(
    entity, n =>
    {
        n.Entry.State = entity.IsNew ? EntityState.Added : EntityState.Modified;
    });

The problem happens when the context is already tracking a principal and dependent when this code is executed, while the principal and dependent passed in are not tracked. This means that the existing principal is detached from the context, but the existing dependent remains tracked. Then, in TrackGraph, when the new principal is tracked it is fixed up to the existing dependent, severing the relationship to the new dependent.

Probably in this case it would be better not to sever this relationship, and instead let an exception be thrown when the attempt is made to track the new dependent. However, given this is a negative case anyway, it's not clear how much value there is in this. It may also prove tricky, since changing fixup behavior for one scenario can often regress a different scenario.

[camainc]

So I am doing it wrong, but what is the correct way to update the graph of a detached entity?

If I comment out the code

if (_dbContext.Entry(entity).State == EntityState.Detached)
{
// get and detach existing entry so we can reattach the updated entity
var existingEntity = _dbSet.IgnoreQueryFilters().Single(x => x.Id == entity.Id);
_dbContext.Entry(existingEntity).State = EntityState.Detached;
}

I get the error "The instance of type '' cannot be tracked because there is another instance with the same value for '{Id}' already being tracked..."

[ajcvickers]

@camainc The best place to start is probably the docs here: https://docs.microsoft.com/en-us/ef/core/saving/disconnected-entities

[camainc]

Thanks ~CRC~ *"You can give without loving, but you cannot love without giving." -- Amy Carmichael* [+] You're welcome to add me to your address book <https://ws.evercontact.com/kwaga-bin/titan/WEB/me.pl/3189554321400600699/i>

…

On Wed, Jul 25, 2018 at 12:38 PM Arthur Vickers ***@***.***> wrote: @camainc <https://github.com/camainc> The best place to start is probably the docs here: https://docs.microsoft.com/en-us/ef/core/saving/disconnected-entities — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#12590 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA5p3stcr-7722ythC8G8Q_Gr9hpHnT4ks5uKMlSgaJpZM4VF8TU> .

[camainc]

So I have removed the code that loaded the existing entity and set it's state to detached. Now I am doing only what is shows on the page you referenced.

My repository update method looks like this:

        public void Update(TEntity entity)
        {
            if (entity == null)
                throw new InvalidOperationException("Entity to update cannot be null.");

            _dbContext.InsertOrUpdateGraph(entity);
        }

And the InsertOrUpdateGraph method in my dbContext class looks like this:

public static void InsertOrUpdateGraph(object rootEntity)
{
    Update(rootEntity);
    SaveChanges();
}

My test method looks like this:

[Test]
        public void TestMethod()
        {
            var options = new DbContextOptionsBuilder<TrackChangesContext>()
                .UseInMemoryDatabase(Guid.NewGuid().ToString())
                .Options;

            const string newDescription = "This is a changed description";

            using (var dbContext = new TrackChangesContext(options))
            {
                LoadTestData(dbContext);

                var repo = new Repository<SampleEntity>(dbContext);

                // gets the entity with AsNoTracking()
                var item = repo.GetById(1, include: i => i.Include(x=>x.SampleLookup));

                // update the item's properties
                item.Description = newDescription;
                
                // update the child's properties
                item.SampleLookup.Description = newDescription;

                // save the changes
                repo.Update(item);

                Assert.That(item.Description, Is.EqualTo(newDescription));
            }
        }

When execution gets to the line in the InsertOrUpdateGraph method, it fails on the Update(rootEntity) statement with this error:

System.InvalidOperationException: 'The instance of entity type 'SampleEntity' cannot be tracked because another instance with the same key value for {'SampleEntityId'} is already being tracked. When attaching existing entities, ensure that only one entity instance with a given key value is attached. Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see the conflicting key values.'

[camainc]

P.S. I have no idea why the formatting gets so messed up. I'm clicking on the "Insert Code" button before pasting the code, and making sure the code goes in between the hash marks.

[ajcvickers]

@camainc If I remember correctly from looking at your code earlier, this is likely because LoadTestData loads entities into the context. So when you try to track the new entity instance it conflicts with the instance that was previously loaded into the context.

[camainc]

By the way, I also tried doing this, from that same article:

var itemToUpdate = repo.GetById(1, i => i.Include(x => x.SampleLookup), asNoTracking: false); dbContext.Entry(itemToUpdate).CurrentValues.SetValues(item);

And that works fine for the root entity, but how would I handle updating the child entities in a generic fashion where I don't know what those children might be? The example in the article shows, in the method named "InsertUpdateOrDeleteGraph(BloggingContext context, Blog blog)" how to update child entities, but using that method I would have to write a custom save method for every entity in my dbContext, and that doesn't seem like a good practice.

[camainc]

Ok, I just read your response from 4:37 PM. Now I am really confused. If I don't add the test data to the in-memory context, how can I test retrieving an entity using AsNoTracking() and then later updating it?

[ajcvickers]

@camainc The in-memory database will store data saved from a DbContext instance into a database that is not persisted to disk. This can then be queried into a new context instance from the in-memory database. In other words, entities being tracked by a context is different from entities being stored in the database, in-memory or not. Generally, it is recommended that the a context instance be used for a single unit-of-work. So in this case, the code that sets up and saves the data into the in-memory database should use on context instance which is then disposed. Then a second context instance should be used for the test that pulls data from the in-memory database.

[camainc]

Ah, ok. That makes sense. I thought since it was all in memory it all had to be in the same context.

So as long as I use the same name for the in-memory database for each new context, it should all work.

I'll give that a try. Thanks for being patient with all of my questions.