Can't insert using EF Core a new entity having already existing navigation property

Question

I'm importing a huge file, which make me create a lots of modules with their Products in a 1-to-N relationship.

However, there can be errors (anomalies), and those anomalies are in a N-to-N relationship with the products:

public class Module 
{
    public int Id { get; set; }

    public ICollection<Product> Products { get; set; } = new List<Product>();
}

public class Product 
{
    public int Id { get; set; }

    public int ModuleId { get; set; }
    public Module? Module { get; set; }

    public ICollection<Anomalie> Anomalies { get; set; } = new List<Anomalie>();
}

public class Anomalie
{
    public int Id { get; set; }
    public ICollection<Product> Products { get; set; } = new List<Product>();
}

It's important to note that the anomalies already exist in the database, and I won't be creating new ones, only link them to the products.

Here I have a generic AddBulkAsync method in which I send my entity in order to insert it :

public virtual async Task<IList<T>> AddBulkAsync(ICollection<T> entities)
{
    try
    {
        await _dbContext.Set<T>().AddRangeAsync(entities);
        await _dbContext.SaveChangesAsync();
    
        _logger.LogInformation($"Entité ajoutée avec succès - {typeof(T).Name}");
        return entities.ToList();
    }
    catch (Exception ex)
    {
        _logger.LogError($"Impossible d'ajouter l'entité {typeof(T).Name} : {ex.Message}");
    }
   
    return new List<T>();
}

Briefly, let's say I have a list of only one entity which I insert :

var modulesToInsert = new List<Module>() {
     new Module() {
        Products = new List<Product>() {
            new Product() { ... },
            new Product() { ... },
            new Product() { ... },
        }
    }
};

await AddBulkAsync(moduleToInsert); // This works

// ----------------------------------------------------------
var anomalie1 = await GetAnomalieWithIdAsync(1); //get from db
var anomalie2 = await GetAnomalieWithIdAsync(2); //get from db

var modulesToInsert2 = new List<Module>() {
     new Module() {
        Products = new List<Product>() {
            new Product() { Anomalies  = new List<Anomalie>() { anomalie1, anomalie2 } },
            new Product() { Anomalies  = new List<Anomalie>() { anomalie1, anomalie2 } },
        }
    }
};

await AddBulkAsync(moduleToInsert2); // This doesn't works

Here I get an error telling me that I the entity Anomalie with Id = 1 is already being tracked.

I tried to create an empty object Anomalie with Id = 1, but I got the same error.

I also need to keep using AddRange because I can have a LOT of entities to insert, any suggestion please?

Answer 1

Firstly, when it comes to EF entities, do not use Generic patterns like the Generic Repository. Generics work on the principle that their behaviour is 100% independent of concerns about what type they are wrapping. It should be clear that the behaviour of some classes will be different for a given operation than others. Some classes will have a requirement to deal with navigation properties, others do not. I do not recommend diving down a rabbit hole with conditional logic in an attempt to find a Generic solution to save a few lines of simple EF code.

Regarding references to existing entities, yes, these absolutely do need to be handled specially. Though from your example I suspect this isn't actually the code you are running because it probably would work the way it is written. I would look at what this GetAnomalieWithIdAsync(1) method is actually doing. For code like this to work you need to ensure two things:

1. That GetAnomalieWithIdAsync is using the same DbContext instance as the code above will be using to insert the product.
2. That GetAnomalieWithIdAsync is not using AsNoTracking or otherwise detaching the read Anomalie.

This error more commonly comes up when developers pass a detached or deserialized entities into the method and just associate them to the new product to be saved without considering that the DbContext is not tracking those instances. The DbContext does not know if an entity referenced represents an existing record in the database or should be treated as a new item associated with the item being added unless it happens to be tracking that entity. It does not make assumptions based on whether the entity has a PK set or is default for instance.

You want to ensure that if you are referencing the same existing entity instance across multiple products, that the exact same reference to that entity is used across all products. For instance:

// won't work:
products.Add(new Product { Anomalies = new[] { New Anomalie { Id = 1 } }.ToList());
products.Add(new Product { Anomalies = new[] { New Anomalie { Id = 1 } }.ToList());

// better:
var anomalie = new Anomalie { Id = 1 };
products.Add(new Product { Anomalies = new [] { anomalie });
products.Add(new Product { Anomalies = new [] { anomalie });

However since that anomalie represents an existing record, we need to ensure that the DbContext is tracking it. This either means loading it from the DbContext instance we are using to insert these products:

var anomalie = await _dbContext.Anomalies.SingleAsync(x => x.Id == 1);
product.Anomaies.Add(anomalie);    

... or associating a "stub" or detached instance, so long as the DbContext isn't already tracking a reference. For instance if our method passed in a detached Anomalie reference:

var existingAnomalie = _dbContext.Anomalies.Local.FirstOrDefault(x => x.Id == anomalie.Id);
if (existingAnomalie != null)
    anomalie = existingAnomalie;
else
    _dbContext.Attach(anomalie);

product.Anomaies.Add(anomalie);    

Essentially whenever given a detached reference we go to the .Local tracking cache to see if the DbContext is tracking a reference. If so, we should use that reference. If not, then we can attach this reference and use it.

One important tip: When you have a class with a collection-based navigation property, make the setter protected to avoid code like:

product.Anomalies = new List()

When constructing a new product, something like that is harmless, but if it gets used on an existing product entity reference it will bung things up with EF.

public class Product 
{
    public int Id { get; set; }

    public int ModuleId { get; set; }
    [Required]
    public Module? Module { get; set; }

    public ICollection<Anomalie> Anomalies { get; protected set; } = new List<Anomalie>();
}

I recommend using constructors or factory methods for building entities especially when they have required relations. You can have a protected default constructor to keep EF happy.

public Product(Module module, params Anomalie[] anomalies)
{
    Module = module;
    ModuleId = module.Id;
    Anomalies = anomalies.ToList();
}

// Available for EF
protected Product() {}

If using a constructor like this to pass Anomalie references, you first need to ensure that the DbContext is happily tracking them to avoid errors like what you have encountered.

Edit: Updating entities with references means that you need to be careful with references. Just like creating two instances representing the same record can cause problems, having two detached/deserialized instances for the same record poses problems with how EF treats those references. First what we cannot do:

public void Update(Product product)
{
    _context.Attach(product);
    _context.Entry(product).State = EntityState.Modified;
    _context.SaveChanges();
}

The problems here:

1. The DbContext may already be tracking a Product instance with the same ID, in which case the Attach call fails at runtime.
2. Every Anomalie associated with the product will untracked and will be treated as an Add, even though we are doing an update to the product. This will result in duplicate rows (with new IDs) or PK violations depending on whether the PK is set up as an Identity column or not.

The next thing that won't work:

public void Update(Product product)
{
    var existingProduct = _context.Products
        .Include(x => x.Anomalies)
        .Single(x => x.Id == product.Id);

    existingProduct.Anomalies = null;
    existingProduct.Anomalies = product.Anomalies;
    _context.SaveChanges();
}

Here the intention might be to remove all existing anomalies from the product and replace them with the set passed in. This doesn't work, and we should never replace an entity's collection references, in fact the setter should be marked as protected to deter code like this. Setting the collection to null does not delete the existing anomalies, and we will end up with the same problem with duplicate rows being added.

To update the associations we need to determine which need to be added and which need to be removed.

public void Update(Product product)
{
    var existingProduct = _context.Products
        .Include(x => x.Anomalies)
        .Single(x => x.Id == product.Id);

    var existingAnomalyIds = existingProduct.Anomalies.Select(x => x.Id).ToList();
    var updatedAnomalyIds = product.Anomalies.Select(x => x.Id).ToList();

    var anomalyIdsToAdd = updatedAnomalyIds.Except(existingAnomalyIds);
    var anomalyIdsToRemove = existingAnomalyIds.Except(updatedAnomalyIds);

    foreach(var anomalyId in anomalyIdsToRemove)
        existingProduct.Anomalies.Remove(existingProduct.Anomalies.First(x => x.Id == anomalyId));

    if(anomalyIdsToAdd.Any())
    {
        var anomaliesToAdd = _context.Anomalies
            .Where(x => anomalyIdsToAdd.Contains(x.Id))
            .ToList();
        foreach(var anomaly in anomaliesToAdd)
             existingProduct.Anomalies.Add(anomaly);
    }

    _context.SaveChanges();
}

Using the IDs we determine which items need to be removed and remove those from the existing tracked entity. Then we get the new items to add from the DbContext and associate those to the tracked entity, then can call SaveChanges(). If we pass a ProductViewModel into this method, that view model just needs to include the set of AnomalyIds rather than entire anomaly entities or view models meaning we can have a more compact block of data sent over the wire to the server.