The instance of entity type cannot be tracked because another instance of this type is already being-CodePudding

I am trying to update an object and am getting an error when Update is called.

public async Task UpdateAsync(int id, CompoundIngredient CompoundIngredient)
        {
            try
            {
                if (!await ExistsAsync(id))
                    return;

                //Update Values and MeasuredIngredients
                _dataContext.CompoundIngredients.Update(CompoundIngredient);

                //Remove Ingredients
                var existingCompoundIngredient = await GetByIdAsync(id);
                foreach( var existingMeasuredIngredient in existingCompoundIngredient.MeasuredIngredients)
                {
                    if (!CompoundIngredient.MeasuredIngredients.Any(c => c.Id == existingMeasuredIngredient.Id))
                        _dataContext.MeasuredIngredients.Remove(existingMeasuredIngredient);
                }
                
                await _dataContext.SaveChangesAsync();
            }
            catch (Exception ex)
            {
                _logger.LogError(ex, "Error in UpdateAsync()");
                throw;
            }
        }

When Update is called I get the error that says "The instance of entity type 'IngredientType' cannot be tracked because another instance of this type with the same key is already being tracked"

The CompoundIngredient looks like the following

I tried adding NoTracking to the Context in the Startup.cs

public void ConfigureServices(IServiceCollection services)
        {
            //Register the Datacontext and Connection String
            services.AddDbContext<DataContext>(options =>
                options.UseQueryTrackingBehavior(QueryTrackingBehavior.NoTracking)
                .UseSqlServer(Configuration.GetConnectionString("DefaultConnection"))
            );

I also tried in the DataContext itself

public class DataContext : ApiAuthorizationDbContext<ApplicationUser>
    {
        public DataContext(DbContextOptions options, IOptions<OperationalStoreOptions> operationalStoreOptions) : base(options, operationalStoreOptions)
        {
            ChangeTracker.QueryTrackingBehavior = QueryTrackingBehavior.NoTracking;
        }

The CompoundIngredient has an IngredientType object on it as well as a MeasuredIngredients List. Each of those MeasuredIngredients contain an IngredientType object as well.

I really do not want to loop through every MeasuredIngredient and remove the object but keep the ID.

What can I do to stop this error

CodePudding user response：

The simple answer is "don't update entity graphs this way".

This issue often happens when you deserialize previously detached entities that have many-to-one reference associations. For example if I have a record that has two ingredients that share the same ingredient-type.

When you read this data from a DbContext, if the CompoundIngredient and associated MeasuredIngredient all reference an IngredientType ID 4, then they will all be given a single Reference to an IngredientType entity for ID 4. The issue is when/if you deserialize that data from something like a View that you sent the CompountIngredient and associated entities/references to. When those entities get de-serialized where each refers to IngredientType ID 4, you will get new individual object references for each reference to IngredientType ID 4. When you try to Attach the objects using Update the first instance of IngredientType ID 4 gets attached but then you will get exceptions when it tries to deal with the additional unique instances that also reference ID 4.

The best suggestion is to not fall into the trap of sending Entities between controllers and views, especially when you have many-to-one references. Turning off change tracking just compounds the problem as this will prevent tracking instances when you read, but doesn't help you when you update. Working with detached entities is messy and even when you do everything correctly it is still error prone as it can and will overwrite data you probably don't intend to be overwritten, and it's easy to miss something that can end up only showing up in certain scenarios, often detected after a production release.

At a bare minimum if you need to continue with updating detached object graphs you need to check for every associated entity to see if it is being tracked and replace references. (updating afterwards if necessary)

The code to check and replace references would be something like this:

var trackedIngredientType = _dataContext.IngredientTypes.Local.SingleOrDefault(x => x.Id == CompoundIngredient.IngredientType.Id);
if (trackedIngredientType != null)
    CompoundIngredient.IngredientType = trackedIngredientType;

foreach(var measuredIngredient in CompoundIngredient.MeasuredIngredients)
{
    trackedIngredientType = _dataContext.IngredientTypes.Local.SingleOrDefault(x => x.Id == measuredIngredient.IngredientType.Id);
    if (trackedIngredientType != null)
        measuredIngredient.IngredientType = trackedIngredientType;
}

_dataContext.CompoundIngredients.Update(CompoundIngredient);

Basically anywhere you have references in an object to be updated, you need to check the .Local tracking cache. If you find a match you need to replace the reference to be updated with the single tracked reference.

The next issue is with code like this done after calling Update:

//Remove Ingredients
var existingCompoundIngredient = await GetByIdAsync(id);
foreach( var existingMeasuredIngredient in existingCompoundIngredient.MeasuredIngredients)
{
    if (!CompoundIngredient.MeasuredIngredients.Any(c => c.Id == existingMeasuredIngredient.Id))
        _dataContext.MeasuredIngredients.Remove(existingMeasuredIngredient);
}

Here you seem to want to load the compound ingredient from the DB anyways, which if GetByIdAsync is using the same _dataContext instance would just return the tracked instance you called Update with.

Realistically an update process for an existing entity should read the "real" entity from the DB first, then inspect the model (whether a ViewModel or a detached entity graph) coming from the View and make appropriate changes to the real, tracked entity. Then there is no reason to attempt to Update the detached, and problematic entity.

The suggested change I would make to the method would be something like:

public async Task UpdateAsync(int id, CompoundIngredient CompoundIngredient)
{
    try
    {
         // Make sure this does a `.Include() for the MeasuredIngredients and all IngredientType references.
         var existingCompoundIngredient = await GetByIdAsync(id);

         // TODO: Copy values from CompoundIngredient to existingCompoundIngredient that might have changed.

         // If the IngredientType changed, fetch an instance and update the reference.
         if (CompoundIngredient.IngredientType.Id != existingCompoundIngredient.IngredientType.Id)
         {
             var ingredientType = _dbContext.IngredientTypes.Single(x => x.Id == CompoundIngredient.IngredientType.Id);
             existingCompoundIngredient.IngredientType = ingredientType;
         }
 
         var existingIngredientIds = existingCompoundIngredient.MeasuredIngredients.Select(x => x.Id).ToList();
         var updatedIngredientIds = CompoundIngredient.MeasuredIngredients.Select(x => x.Id).ToList();
         var ingredientIdsToRemove = existingIngredientIds.Except(updatedIngredientIds).ToList();
         var ingredientIdsToAdd = updatedIngredientIds.Except(existingIngredientIds).ToList();

         if(ingredientIdsToRemove.Any())
         {
              var ingredientsToRemove = existingCompoundIngredient.MeasuredIngredients.Where(x => ingredientIdsToRemove.Contains(x.Id)).ToList();
              foreach(var ingredient in ingredientsToRemove)
                  existingCompountIngredient.MeasuredIngredients.Remove(ingredient);
         }

         if(ingredientIdsToAdd.Any())
         {
             // Here we can create new MeasuredIngredient instances to add to the existing compound, or add our detached record, but only after replacing the IngredientType with a tracked reference.

             var ingredientsToAdd = CompoundIngredient.MeasuredIngredients.Where(x => ingredientIdsToAdd.Contains(x.Id)).ToList();
             var ingredientTypeIds = ingredientsToAdd.Select(x => x.IngredientType.Id).ToList();
             var ingredientTypes = _dbContext.IngredientTypes.Where(x => ingredientTypeIds.Contains(x.Id)).ToList(); // Fetch all used ingredient types in one call.
             foreach(var ingredient in ingredientsToAdd)
             {
                 ingredient.IngredientType = ingredientTypes.Single(x => x.Id == ingredient.IngredientType.Id); // Replace references with a read tracked instance.
                 existingCompoundIngredient.MeasuredIngredients.Add(ingredient);
             }
         }

         await _dataContext.SaveChangesAsync();
    }
    catch (Exception ex)
    {
         _logger.LogError(ex, "Error in UpdateAsync()");
         throw;
    }
}

The basic gist is that the data coming into your call are not "entities" in the sense that they will be recognized as complete or trackable references for a DbContext. You should read the existing data state, validate the provided details against that data state and copy valid details across. When dealing with graphs of related entities you need to inspect and handle scenarios where associations may be added or removed, but always by dealing with tracked references. Attaching untracked references can appear to work fine in some scenarios, but lead to errors in others when it encounters two references intended to point to the same record in the DB.