Another instance of an entity raises an error when update

Question

In my ASP.NET Core 9 Web API and Entity Framework Core, I have a model class for a Client that has a relationship many to many with the model class Channel.

public class Client
{
    [Key]
    public long ID { get; set; }
}

public class Channel
{
    [Key]
    public long ID { get; set; }
    public string? Name { get; set; }
    public ICollection<Client>? Client { get; set; }
}

After the migration, the database has the structure of a new table as I expect:

I can add new values using Entity Framework Core.

The problem starts when I want to update the values.

I have an API, with a PUT verb to be precise, that receives the Client object as a parameter with all the details. First I read the object from the database including the Channels:

var localClient = await db.Client.AsNoTracking()
                          .Include(c => c.Channels)
                          .FirstOrDefaultAsync(model => model.ID == id);

Then, I map the parameter with the data from the database:

localClient = mapper.Map<Domain.Client>(client);

And then I update the Channels using the values from the parameter:

localClient.Channels?.Clear();

if (client.Channels != null)
{
    var listChannels = client.Channels.ToList();

    foreach (Channel ch in listChannels)
    {
        var l = await db.Channels.Where(c => c.ID == ch.ID).FirstOrDefaultAsync();

        if (l != null)
            if (localClient.Channels!.Count(c => c.ID == l.ID) == 0)
                localClient.Channels?.Add(l);
    }
}

If I inspect the localClient object, there are only unique channels and no duplication. When I want to save using

db.Attach(client);

I immediately get this error:

The instance of entity type 'Channel' cannot be tracked because another instance with the same key value for {'ID'} 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.

I can't understand why I get this error. I checked my old projects and I use a similar process.

Update

client is an object that I pass via API: this contains all the details about the client, also the list of channels.

group.MapPut("/{id}",
        async Task<Results<Ok, NotFound>> (long id, Domain.Client client, 
        MyDbContext db, IMapper mapper) =>
        {
            // code above
        }

I fetch the client from the database because I was thinking that the error was related to a new instance or record instead of updating an existing one.

Update/2

I created on GitHub a small project to test the update. I applied the suggestions below but the object is not updated on the database.

Answer 1

Avoid using Clear and attempting to link the items back in. Also your loading and mapping aren't doing what you probably think they are doing. This code:

var localClient = await db.Client
                          .Include(c => c.Channels)
                          .(model => model.Id == id);

localClient = mapper.Map<Domain.Client>(client);

mapper.Map() will replace the reference, making the call to fetch the local client completely pointless. What you want instead /w Automapper is:

mapper.Map(client, localClient);

This tells Automapper to copy values from the DTO (client passed in) into the loaded local client. The mapping configuration used to copy values across should ignore collection-based navigation properties where you want to associate/disassociate references. Automapper won't be able to handle these automatically.

Also avoid the crutch of OrDefault() unless you are prepared for, and handle the possibility of nothing coming back. When querying the database (Linq-to-DB) use .Single() or .First() is fine if querying by PK. When querying across in-memory sets, including the .Local tracking cache, use .First(). If a record isn't found we get an exception on that line rather than a NullReferenceException somewhere later on where there could be multiple references that might be #null unexpectedly.

When it comes to handling the association and disassociation of channels, you should do this by adding and subtracting references from the set as needed. The Clear() and AddRange() approach will lead to reference problems.

var localClient = await db.Client
    .Include(c => c.Channels)
    .FirstAsync(model => model.Id == id);
mapper. Map(client, localClient);

var updatedChannelIds = client.Channels.Select(c => c.Id).ToList();
var currentChannelIds = localClient.Channels.Select(c => c.ID).ToList();
var channelIdsToAdd = updatedChannelIds.Except(currentChannelIds);
var channelIdsToRemove = currentChannelIds.Except(updatedChannelIds);

if (channelIdsToRemove.Any())
{
    var channelsToRemove = localClient.Channels
                           .Where(c => channelIdsToRemove.Contains(c.Id))
                           .ToList();
    foreach(var channel in channelsToRemove)
        localClient.Channels.Remove(channel);
}
if (channelIdsToAdd.Any())
{
    var channelsToAdd = await db.Channels
                              .Where(c => channelIdsToAdd.Contains(c.Id))
                              .ToListAsync();
    foreach(var channel in channelsToAdd)
        localClient.Channels.Add(channel);
}

await db.SaveChangesAsync();

References are everything with EF so in your original code you were mapping a new, detached instance of Client, including Channels using the Mapper, then clearing the channels. This does not remove the channel references from the tracking cache so adding new detached copies leads to tracking errors.