6

I am basically trying to implement CRUD using EntityFrameWork core and .Net core 3.1. I have an issue with my update operation where I am not able update the context with the modified value. I am using postman to initiate the request.

As you can see in the code below, I am trying to check if that customer exist and if it does pass the modified object to the context.

enter image description here

Function code

   [FunctionName("EditCustomer")]
    public async Task<IActionResult> Run(
    [HttpTrigger(AuthorizationLevel.Anonymous,"post", Route = "update-customer")] HttpRequest req)
    {
        var customer = JsonConvert.DeserializeObject<CustomerViewModel>(new StreamReader(req.Body).ReadToEnd());
        await _repo.UpdateCustomer(customer);
        return new OkResult();
    }

Repository method

  public async Task UpdateCustomer(CustomerViewModel customerViewModel)
    {
        if (customerViewModel.CustomerId != null)
        {
            var customer = _context.Customers.Where(c => c.CustomerId.Equals(customerViewModel.CustomerId)).FirstOrDefault();

            if (customer == null)
            {
                throw new Exception("customer not found");
            }
            else
            {
                _context.Customers.Update(_mapper.Map<Customers>(customerViewModel));
                await _context.SaveChangesAsync();

            }
        }
       
    }

Mapping

   public class CustomerManagerProfile : Profile
    {
        public CustomerManagerProfile()
        {
            CreateMap<CustomerDetails, CustomerDetailsViewModel>().ReverseMap();
            CreateMap<CustomerOrders, CustomerOrdersViewModel>().ReverseMap();
            CreateMap<CustomerOrderDetails, OrderDetailsViewModel>().ReverseMap();
            CreateMap<Customers, CustomerViewModel>().ReverseMap();
        }
    }

Solution

public async Task UpdateCustomer(CustomerViewModel customerViewModel)
    {
        if (customerViewModel.CustomerId != null)
        {
            var customer = _context.Customers.Where(c => c.CustomerId.Equals(customerViewModel.CustomerId)).FirstOrDefault();

            if (customer == null)
            {
                throw new Exception("customer not found");
            }
            else
            {
                var customerModel = _mapper.Map<Customers>(customerViewModel);
                _context.Entry<Customers>(customer).State = EntityState.Detached;
                _context.Entry<Customers>(customerModel).State = EntityState.Modified;
                await _context.SaveChangesAsync();

            }
        }
}
  

     
Matt
  • 25,467
  • 18
  • 120
  • 187
Tom
  • 8,175
  • 41
  • 136
  • 267
  • 1
    Does this answer your question? [The instance of entity type cannot be tracked because another instance of this type with the same key is already being tracked](https://stackoverflow.com/questions/36856073/the-instance-of-entity-type-cannot-be-tracked-because-another-instance-of-this-t) – Hammas Jul 16 '20 at 11:47
  • After reading the post i have implemented the solution. Update my post. It works . Could you let me know if its the right way to do it – Tom Jul 16 '20 at 12:57
  • Yes it's right now. Also you can use `FirstOrDefaultAsync()` – Hammas Jul 16 '20 at 13:08
  • When I use FirstOrDefaultAsync it complains cannot convert from 'System.Threading.Tasks.Task' to 'SRL.Data.Models.Customers' SRL.CustomerManager – Tom Jul 16 '20 at 13:14
  • @Tom I edited my answer to comment on solution that you provide. I hope that makes it clearer on what is going on. – Marchyello Jul 16 '20 at 13:15
  • @Tom you get the error, because you don't await your async call. – Marchyello Jul 16 '20 at 13:16
  • @Tom whenever you will use `Async` method you will need to use `await` like this `var customer = await _context.Customers.Where(c => c.CustomerId.Equals(customerViewModel.CustomerId)).FirstOrDefault();` – Hammas Jul 16 '20 at 13:17
  • just like you are using await here `await _context.SaveChangesAsync();` – Hammas Jul 16 '20 at 13:18
  • Rao, I think Marchyello's recommendation makes sense and is leaner. What is your opinion – Tom Jul 16 '20 at 13:20
  • 1
    never ever map from VM to entity. Automapper was created to map from Entity to VM, VM to DTO or DTO(data trasfer object) to VM(view model) – zolty13 Jul 16 '20 at 13:51

3 Answers3

13

Entity Framework tracks your entities for you. For simplicity's sake, think of it like keeping a dictionary (for every table) where the dictionary key is equal to your entity's PK.

The issue is that you can't add two items of the same key in a dictionary, and the same logic applies to EF's change tracker.

Let's look at your repository:

var customer = _context
                  .Customers
                  .Where(c => c.CustomerId.Equals(customerViewModel.CustomerId))
                  .FirstOrDefault();

The fetched customer is retrieved from the database and the change tracker puts it in his dictionary.

var mappedCustomer = _mapper.Map<Customers>(customerViewModel);
_context.Customers.Update();

I split your code in two steps for the sake of my explanation.

It's important to realize that EF can only save changes to tracked objects. So when you call Update, EF executes the following check:

  • Is this the same (reference-equal) object as one I have I my change tracker?
  • If yes, then it's already in my change tracker.
  • If not, then add this object to my change tracker.

In your case, the mappedCustomer is a different object than customer, and therefore EF tries to add mappedCustomer to the change tracker. Since customer is already in there, and customer and mappedCustomer have the same PK value, this creates a conflict.

The exception you see is the outcome of that conflict.

Since you don't need to actually track your original customer object (since EF doesn't do anything with it after fetching it), the shortest solution is to tell EF to not track customer:

var customer = _context
                  .Customers
                  .AsNoTracking()
                  .Where(c => c.CustomerId.Equals(customerViewModel.CustomerId))
                  .FirstOrDefault();

Since customer is now not put into the change tracker, mappedCustomer won't cause a conflict anymore.

However, you don't actually need to fetch this customer at all. You're only interested in knowing whether it exists. So instead of letting EF fetch the entire customer object, we can do this:

bool customerExists = _context
                        .Customers
                        .Any(c => c.CustomerId.Equals(customerViewModel.CustomerId));

This also solves the issue since you never fetch the original customer, so it never gets tracked. It also saves you a bit of bandwidth in the process. It's admittedly negligible by itself, but if you repeat this improvement across your codebase, it may become more significent.

Flater
  • 12,908
  • 4
  • 39
  • 62
4

The most simple adjustment that you could make would be to avoid tracking your Customers on retrieval like this:

var customer = _context
    .Customers
    .AsNoTracking() // This method tells EF not to track results of the query.
    .Where(c => c.CustomerId.Equals(customerViewModel.CustomerId))
    .FirstOrDefault();

It's not entirely clear from the code, but my guess is your mapper returns a new instance of Customer with the same ID, which confuses EF. If you would instead modify that same instance, your call to .Update() should work as well:

var customer = _context.Customers.Where(c => c.CustomerId.Equals(customerViewModel.CustomerId)).FirstOrDefault();
customer.Name = "UpdatedName"; // An example.
_context.Customers.Update(customer);
await _context.SaveChangesAsync();

As a matter of fact, if you track your Customer you don't even need to explicitly call .Update() method, the purpose of tracking is to be aware of what changes were made to the entities and should be saved to the database. Therefore this will also work:

// Customer is being tracked by default.
var customer = _context.Customers.Where(c => c.CustomerId.Equals(customerViewModel.CustomerId)).FirstOrDefault();
customer.Name = "UpdatedName"; // An example.
await _context.SaveChangesAsync();

EDIT:
The solution you yourself provide begins by tracking the results of your query (the Customer) instance, then stops tracking it (a.k.a. gets detached) before writing to database and instead starts tracking the instance that represents the updated Customer and also marks it as modified. Obviously that works as well, but is just a less efficient and elegant way of doing so.
As a matter of fact if you use this bizarre approach, I don't see the reason for fetching your Customer at all. Surely you could just:

if (!(await _context.Customers.AnyAsync(c => c.CustomerId == customerViewModel.CustomerId)))
{
    throw new Exception("customer not found");
}

var customerModel = _mapper.Map<Customers>(customerViewModel);
_context.Customers.Update(customerModel);
await _context.SaveChangesAsync();
Marchyello
  • 621
  • 2
  • 7
  • 18
  • As you mention modifying the same instance customer.Name = "UpdatedName"; // An example. How do I know at this point which property is modified. If I need to set all the property assignments then whats the role of the mapper. – Tom Jul 16 '20 at 12:44
  • I have just updated the post the way I have tackled it – Tom Jul 16 '20 at 12:54
  • @Tom You're correct that you don't know which properties are different. There's a few approaches you can take: equality checking ("if value is different, set value"), or simply attaching the new object instead of the old one (which does cause all properties to be "changed", but that's usually not a big deal) – Flater Jul 16 '20 at 13:32
0

You use AutoMapper wrong way. It is not created to map from View model or DTO to Entity classes. It makes many problems and you are facing with only one of them now.

If you have more complex bussiness logic in you app (not just udpate all fields), it will be horrible to manage, test and debug what actually is happening in your code. You should write you own logic with some bussiness validation in case when you want to make some other update than CRUD.

If I were you I would create UpdateFields method in Customer class which would update them and finally call SaveChanges. It depends on whether you use anemic entity (anti)pattern or not. If you do not want your entity class to have any method you can create just method which manually map you VM do entity with some domain validation

zolty13
  • 1,943
  • 3
  • 17
  • 34