Hi Guys,
I have encountered some unusual behaviour in the framework and I'd really appreciate some advice in working around it.
I have a service which takes in a csv file, converts to entities and then saves to the db.
For negative testing I was doing the following:
- Purposely import a decimal which is too long for the db field.
- Catch the error, and return a nicely formatted error to the user.
- But the error never returns, instead a 500 Internal error is returned to the user.
I understand from #5997 that this is likely caused by SaveChanges() auto executing upon existing the function. I am catching the internal error but another is then thrown by the auto-execution.
I have tried to use transaction scopes and also a local unit of work but both fail because the operations performed in CreateOrUpdateEnquiry() cannot attach to the local transaction scope. Is there something I am missing here? Is there some way for me to use a local unit of work here so that upon exiting its scope the global context does not contain any unsaved changes?
I do have a working solution as seen below which just scans the context and undoes changes RejectChanges(). But is there a more elegant/efficient way to do this?
Thanks, Barry.
private async Task<ImportedEnquiriesReportDto> CreateEnquiries(List<ImportedEnquiryDto> importedEnquiries)
{
_enquiryDictionary = await GetEnquiryDictionary();
_enquiryStageDictionary = await GetEnquiryStageDictionary();
_enquiryTypeDictionary = await GetEnquiryTypeDictionary();
_surveyTypeDictionary = await GetSurveyTypeDictionary();
var report = new ImportedEnquiriesReportDto();
List<Enquiry> enquiriesToCreate = new List<Enquiry>();
try
{
_ctx.ChangeTracker.AutoDetectChangesEnabled = false; // Get context and disable EF auto-detect changes. HUGE speed increase!!
foreach (var importedEnquiry in importedEnquiries)
{
if (importedEnquiry.CanBeImported())
{
try
{
// New enquiries are created and added to the enquiriesToCreate list.
// Existing enquiries are updated in CreateOrUpdateEnquiry() and picked up by the _ctx.ChangeTracker.DetectChanges() call below.
// Newly created enquiries will have a detached state at this point and hence can be distinguished from updates, unchanged etc.
var enquiry = await CreateOrUpdateEnquiry(importedEnquiry);
if (_ctx.Entry(enquiry).State == EntityState.Detached)
{
enquiriesToCreate.Add(enquiry);
}
}
catch (Exception)
{
report.InvalidEnquiries.Add(importedEnquiry);
}
}
else
{
report.InvalidEnquiries.Add(importedEnquiry);
}
}
// Add new enquiries in bulk. Only call detect changes once after adding all new enquiries.
if (enquiriesToCreate.Any())
{
await _ctx.Enquiries.AddRangeAsync(enquiriesToCreate);
enquiriesToCreate.Clear();
}
// Detect any remaining updates etc.
_ctx.ChangeTracker.DetectChanges();
report.EnquiriesCreated = _ctx.ChangeTracker.Entries<Enquiry>().Count(e => e.State == EntityState.Added);
report.EnquiriesUpdated = _ctx.ChangeTracker.Entries<Enquiry>().Count(e => e.State == EntityState.Modified);
report.EnquiriesUnchanged = _ctx.ChangeTracker.Entries<Enquiry>().Count(e => e.State == EntityState.Unchanged);
await _ctx.SaveChangesAsync();
}
catch (Exception)
{
RejectChanges(); // This works but is there a nicer way??
throw new UserFriendlyException(L("ErrorSavingToDatabase"));
}
finally
{
// Turn EF auto detect changes back on.
_ctx.ChangeTracker.AutoDetectChangesEnabled = true;
}
return report;
}
private void RejectChanges()
{
foreach (var entry in _ctx.ChangeTracker.Entries())
{
switch (entry.State)
{
case EntityState.Modified:
case EntityState.Deleted:
entry.State = EntityState.Modified; //Revert changes made to deleted entity.
entry.State = EntityState.Unchanged;
break;
case EntityState.Added:
entry.State = EntityState.Detached;
break;
}
}
}
6 Answer(s)
-
0
In your catch, if you comment out the throw, does it still happen?
-
0
Unfortuntely yes. If I do not implement RejectChanges(), my initial Catch does indeed catch the issue and set the error message but it never gets returned because the auto-executed SaveChanges() then executes and fails with a 500 error.
I refined the RejectChanges() function to be a bit more optimised also.
// Undo any changes in the db context. private void RejectContextChanges() { var changedEntries = _ctx.ChangeTracker.Entries() .Where(e => e.Entity != null && e.State != EntityState.Unchanged && e.State != EntityState.Detached) .ToList(); foreach (var entry in changedEntries) { switch (entry.State) { case EntityState.Added: entry.State = EntityState.Detached; break; case EntityState.Modified: entry.State = EntityState.Unchanged; break; case EntityState.Deleted: entry.State = EntityState.Modified; //Revert changes made to deleted entity. entry.State = EntityState.Unchanged; break; } } } -
0
Hi,
Thanks for coming back to me. I had also tried to use the local unit of work in the same manner as specified in #5997
using (var uow = unitOfWorkManager.Begin(TransactionScopeOption.RequiresNew)) // Add this scope { ...code goes here await unitOfWorkManager.Current.SaveChangesAsync(); await uow.CompleteAsync(); }But when I do it this way, I then get errors that related properties cannot be created in CreateOrUpdateEnquiry(), the specific error was "The instance of {entity type} cannot be tracked because another instance of this type with the same key is already being tracked".
If you specify the unit of work locally in this manner, I would expect that local unit of work to then also be used by functions called from within the using block. Is that correct?
I also tried a local transaction scope but I then received an error that the "Connection currently has transaction enlisted. Finish current transaction and retry".
Note AutoDetectChangesEnabled is false in my function also.
Thanks.
-
0
FWIW - Have you tried creating DTO objects then firing into a seperate UOW function that maps to your enity then creates?
I've found myself stepping "outside the EF bubble" with DTO objects sometimes, both for speed and tracking issues.
And thanks for the AutoDetectChangesEnabled tip - never tried that.
Speaking of - if you comment that out while debugging - do you get more information?
