1

I am working on an ASP.NET Core application using Entity Framework Core for data access. I am encountering an issue when trying to edit a FiscalYear entity in my database. I have the following code:

public async Task<Response> EditFiscalYear(FiscalYear fiscalYear)
{
    try
    {
        var validate = await EditFiscalYearValidate(new FiscalYearValidationModel { Object = fiscalYear });
        if (validate.StatusCode == System.Net.HttpStatusCode.BadRequest)
            return validate;
_context.EditEntity(fiscalYear);
        await _context.SaveChangesAsync();
        return SuccessResponse(resultObject: fiscalYear, resultText: "Fiscal Year Updated Successfully");
    }
    catch (Exception ex)
    {
        return FaultResponse(ex.Message);
    }
}

And the validation method is:

public async Task<Response> EditFiscalYearValidate(FiscalYearValidationModel model)
{
    if ((await _context.PayPeriods
            .AnyAsync(x => x.FiscalYearId == model.Object.FiscalYearId)))
    {
        return FaultResponse("Fiscal Year Dependency found");
    }

    var fiscalYear = await _context.FiscalYears.FirstOrDefaultAsync(x => x.FiscalYearId == model.Object.FiscalYearId);

    if (fiscalYear is null)
        return NotFoundResponse("No Fiscal Year Found");

    // Other validation checks...

    return SuccessResponse();
}

Here is the EditEntity function for more clarity:

  public void EditEntity<TEntity>(TEntity entity, List<string> excludedParams = null)
        {
            var entry = this.Entry(entity);
            entry.State = EntityState.Modified;

            //some other modifications
        }

When I try to edit a FiscalYear entity, I get the following exception:

"The instance of entity type 'FiscalYear' cannot be tracked because another instance with the same key value for {'FiscalYearId'} 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 understand that EF Core is complaining because it's trying to track two instances of the same entity at the same time. However, I'm puzzled because when I don't store the result of FirstOrDefaultAsync in a variable, I don't get the exception:

await _context.FiscalYears.FirstOrDefaultAsync(x => x.FiscalYearId == model.Object.FiscalYearId);

Why does this happen?

As a solution I am currently doing

 var fiscalYear = _context.FiscalYears.AsNoTracking().FirstOrDefaultAsync(x => x.FiscalYearId == model.Object.FiscalYearId);

This solves my problem but I still don't understand why the tracking behaviour doesn't manifest in the same manner for

await _context.FiscalYears.FirstOrDefaultAsync(x => x.FiscalYearId == model.Object.FiscalYearId);
anas1149
  • 31
  • 3
  • 2
    What is `_context.EditEntity(fiscalYear);`? – Svyatoslav Danyliv Aug 30 '23 at 13:08
  • Taken a look at [this](https://stackoverflow.com/a/48204159/7450780)? – DenisMicheal.k Aug 30 '23 at 13:16
  • @SvyatoslavDanyliv added the EditEntity description – anas1149 Aug 30 '23 at 13:49
  • Well, why you even change stage of the Entry? EF Core automatically tracks changes, so just modify properties and do not touch anything else. – Svyatoslav Danyliv Aug 30 '23 at 13:51
  • @DenisMicheal.k I don't have the same problem, my main question is related to why storing the result of the FirstOrDefaultAsync query in variable causes EFCore to track the entity but if I don't store it then it seemingly doesn't track ? Or even if it does track, doesn't cause a tracking conflict – anas1149 Aug 30 '23 at 13:56
  • @SvyatoslavDanyliv I am building upon legacy work and currently for some reason cannot change this. Our system is already mud of ball microservices, so have to achieve my goals with minimum changes. – anas1149 Aug 30 '23 at 13:59
  • Use tracked entities and do not play with Entries, just modify properties and everything should go smoothly. – Svyatoslav Danyliv Aug 30 '23 at 14:00
  • 2
    *"when I don't store the result of FirstOrDefaultAsync in a variable, I don't get the exception*" No way. Please don't waste your time with meaningless tests. Either use no tracking query, or work with tracked entity once you get it from db. – Ivan Stoev Aug 30 '23 at 15:38

1 Answers1

1

This is the detached entity trap.

public async Task<Response> EditFiscalYear(FiscalYear fiscalYear)

The data passed into this method is merely cast as an entity. It is a detached instance that will not be associated with your DbContext.

Later in your code you are telling the DbContext to fetch an existing FiscalYear:

var fiscalYear = await _context.FiscalYears.FirstOrDefaultAsync(x => x.FiscalYearId == model.Object.FiscalYearId);

if (fiscalYear is null)
    return NotFoundResponse("No Fiscal Year Found");

At this point, the DbContext will be tracking an instance of the entity for that fiscal year.

After this, trying to execute:

var entry = this.Entry(entity);
entry.State = EntityState.Modified;

... passing the "phony" fiscalState entity passed into the method will attempt to attach that phony entity to the DbContext. The context is already tracking a reference, so you get the complaint.

Using AsNoTracking() when querying the FiscalState for the validation will appear to fix the problem, but it isn't fixing anything. You honestly should not be updating data this way.

// _context.EditEntity(fiscalYear); <- never do this with a detached entity you cannot guarantee the quality of.

... Instead: (long way)

var existingFiscalYear = _context.FiscalYears.Single(x => x.FiscalYearId == fiscalYear.FiscalYearId);
existingFiscalYear.SomeAllowedValue = fiscalYear.SomeAllowedValue;
//... copy across all allowed values to update...
_context.SaveChanges();

... or short way with Automapper / mapper:

var existingFiscalYear = _context.FiscalYears.Single(x => x.FiscalYearId == fiscalYear.FiscalYearId);
_mapper.Map(fiscalYear, existingFiscalYear);
_context.SaveChanges();

... where the Mapper is configured with knowing what is allowed to be copied from object to object so only expected data is copied across.

The main problems with doing the EntityState.Modified approach with detached entities, is the already tracking error you are getting, and also that you are trusting the data coming in entirely. If this is something like an API or web Controller method where the data coming in is just serialize values, it can be tampered with by mischievous or malicious users, or bugs in the client-side code leading to values being updated you don't intend to be changed. It's also inefficient as this will always generate an UPDATE statement with all fields whether they changed or not. Editing a tracked instance instead will only generate an UPDATE statement if something changed, and only for the fields that changed.

If you do trust a detached entity and want to update it, you must always check the DbContext's local tracking cache for existing entities in case a reference happens to be attached. If you find one then you either have to detach it first (potentially losing changes) or perform the copy-over-approach. Checking for a tracked instance is done by looking at the .Local of the DbSet:

var trackedInstance = _context.FiscalYears.Local.FirstOrDefailt(x => x.FiscalYearId == fiscalYear.FiscalYearId)

if this returns something, you can copy values across. If it doesn't then you can attach this instance. (Again, provided you can 100% trust the detached instance is complete and correct)

Steve Py
  • 26,149
  • 3
  • 25
  • 43