Entity Framework duplicating records when calling SaveChanges

Question

I am using Entity Framework 6:

JObject jCardRecord = JObject.FromObject(record, _ignoreCircularReferenceJsonSerializer);
JObject jProduct = JObject.FromObject(product, _ignoreCircularReferenceJsonSerializer);

NexusData additionalSwitchData = CreateAdditionalSwitchData(db, job);
updateResponse = eftService.eft.PrePrintEFT(ref jCardRecord, additionalSwitchData);

if (updateResponse.Success)
{
    job.CardRecord = jCardRecord.ToObject<CardRecord>();
    db.MarkAsModified(job);
    db.SaveChanges();
    break;
}

The problem is that when I try to update the job.CardRecord with the jCardRecord that was updated in the method PrePrintEFT as a ref and save changes, the record is duplicated and not only in the cardRecord table but other tables also get a duplicate record.

I need a way to tell EF that cardRecord inside Job was updated and we need to update that record in the database.

I tried attaching the cardRecord but got an error saying that this record already exits.

I really appreciate any help on this. Thanks.

Answer 1

EF DbContext will track references by default so working with detached entities or re-composed object graphs will cause conflicts like this to come up. You need to check the DbContext for any tracked references, and update those references. For example If we have a Job with a CardRecord composed from data where we know that CardRecord will represent an existing row in the database. To do this without unnecessary round trips to the database:

// if Job represents an Update scenario for an existing record..

var trackedCardRecord = _context.CardRecords.Local.SingleOrDefault(c => c.CardRecordId == job.CardRecord.CardRecordId);
if (trackedCardRecord != null)
{
    Mapper.Map(job.CardRecord,trackedCardRecord);
    job.CardRecord = trackedCardRecord;
}
else
    _context.Attach(job.CardRecord);

var trackedJob = _context.Jobs.Local.SingleOrDefault(j => j.JobId == job.JobId);
if (trackedJob != null)
{
    Mapper.Map(job, trackedJob);
    trackedJob.CardRecord = job.CardRecord; // Ensure the tracked job has the tracked card reference.
    job = trackedJob; // Use this tracked reference
}
else
    _context.Attach(job);

_context.Entry(job).State = EntityState.Modified;
_context.Entry(job.CardRecord).State = EntityState.Modified;

_context.SaveChanges();

The above example uses Automapper to copy the values from our passed in copies to the tracked entities if there are any locally tracked references. This can be done manually by copying values from job to trackedJob etc.

Now if that looks like a bit of work to cater for possible tracked references, yeah, it is, and it may still not be enough. It also potentially exposes the system to unintended tampering as we shouldn't want to ever use EntityState.Modified on entities as this performs an UPDATE across all columns whether they have changed or not, or we expect to allow them to change or not. There is also the situation where another user/process could have updated the records in the database since the "copy" that was modified was taken and passed back.

A simpler and more reliable method is to update by fetching the current data state, then copying the allowed modified values across from the ViewModel/DTO:

var existingJob = _context.Jobs
    .Include(j => j.CardRecord)
    .Single(j => j.JobId == job.JobId);

// TODO: Here you can check RowVersion between existingJob and passed in data.

Mapper.Map(job, existingJob);
Mapper.Map(cardRecord, existingJob.CardRecord);

_context.SaveChanges(); 

This would have the Automapper mapper set up to only copy the allowed values that are expected to change. Because we are updating a tracked entity, EF will generate an UPDATE statement for just the values that changed, if they changed. The code is kept a lot simpler which leaves fewer places for bugs to hide. The "TODO" section involves adding a concurrency check if you have something like a row version or timestamp that the DB updates automatically when rows are updated. If the ViewModel/DTO keep a record of a row version, then when you go to do an update, you check if that row version had changed, then handle that scenario. (Such as rejecting the change telling the user that the data has been changed since they started their edit) For instance when you read the record to pass to the client view, it had a RowVersion value of 14. When the data gets sent back to the server with a value of 14, we read the current Job/CardRecord and compare the RowVersion. If still 14 in the DB then nothing had been updated, if different (15, 16, etc.) then someone else has made changes. Row version can be incremented by the DB automatically like sequences, or using something like a Timestamp column.

There is usually some aversion to making a round trip to the database when updating/inserting, but trying to avoid it makes code a lot more complex and error prone. Reading rows by PK/FK is extremely fast/efficient and a lot safer.