Update Entity from ViewModel in MVC using AutoMapp

问题:

I have a Supplier.cs Entity and its ViewModel SupplierVm.cs. I am attempting to update an existing Supplier, but I am getting the Yellow Screen of Death (YSOD) with the error message:

The operation failed: The relationship could not be changed because one or more of the foreign-key properties is non-nullable. When a change is made to a relationship, the related foreign-key property is set to a null value. If the foreign-key does not support null values, a new relationship must be defined, the foreign-key property must be assigned another non-null value, or the unrelated object must be deleted.

I think I know why it is happening, but I'm not sure how to fix it. Here's a screencast of what is happening. I think the reason I'm getting the error is because that relationship is lost when AutoMapper does its thing.

CODE

Here are the Entities that I think are relevant:

public abstract class Business : IEntity
{
  public int Id { get; set; }
  public string Name { get; set; }
  public string TaxNumber { get; set; }
  public string Description { get; set; }
  public string Phone { get; set; }
  public string Website { get; set; }
  public string Email { get; set; }
  public bool IsDeleted { get; set; }
  public DateTime CreatedOn { get; set; }
  public DateTime? ModifiedOn { get; set; }
  public virtual ICollection<Address> Addresses { get; set; } = new List<Address>();
  public virtual ICollection<Contact> Contacts { get; set; } = new List<Contact>();
}

public class Supplier : Business
{
  public virtual ICollection<PurchaseOrder> PurchaseOrders { get; set; }
}

public class Address : IEntity
{
  public Address()
  {
    CreatedOn = DateTime.UtcNow;
  }

  public int Id { get; set; }
  public string AddressLine1 { get; set; }
  public string AddressLine2 { get; set; }
  public string Area { get; set; }
  public string City { get; set; }
  public string County { get; set; }
  public string PostCode { get; set; }
  public string Country { get; set; }
  public bool IsDeleted { get; set; }
  public DateTime CreatedOn { get; set; }
  public DateTime? ModifiedOn { get; set; }
  public int BusinessId { get; set; }
  public virtual Business Business { get; set; }
}

public class Contact : IEntity
{
  public Contact()
  {
    CreatedOn = DateTime.UtcNow;
  }

  public int Id { get; set; }
  public string Title { get; set; }
  public string FirstName { get; set; }
  public string LastName { get; set; }
  public string Phone { get; set; }
  public string Email { get; set; }
  public string Department { get; set; }
  public bool IsDeleted { get; set; }
  public DateTime CreatedOn { get; set; }
  public DateTime? ModifiedOn { get; set; }

  public int BusinessId { get; set; }
  public virtual Business Business { get; set; }
}

And here is my ViewModel:

public class SupplierVm
{
  public SupplierVm()
  {
    Addresses = new List<AddressVm>();
    Contacts = new List<ContactVm>();
    PurchaseOrders = new List<PurchaseOrderVm>();
  }

  public int Id { get; set; }
  [Required]
  [Display(Name = "Company Name")]
  public string Name { get; set; }
  [Display(Name = "Tax Number")]
  public string TaxNumber { get; set; }
  public string Description { get; set; }
  public string Phone { get; set; }
  public string Website { get; set; }
  public string Email { get; set; }
  [Display(Name = "Status")]
  public bool IsDeleted { get; set; }

  public IList<AddressVm> Addresses { get; set; }
  public IList<ContactVm> Contacts { get; set; }
  public IList<PurchaseOrderVm> PurchaseOrders { get; set; }

  public string ButtonText => Id != 0 ? "Update Supplier" : "Add Supplier";
}

My AutoMapper mapping configuration is like this:

cfg.CreateMap<Supplier, SupplierVm>();
cfg.CreateMap<SupplierVm, Supplier>()
  .ForMember(d => d.Addresses, o => o.UseDestinationValue())
  .ForMember(d => d.Contacts, o => o.UseDestinationValue());
cfg.CreateMap<Contact, ContactVm>();
cfg.CreateMap<ContactVm, Contact>()
  .Ignore(c => c.Business)
  .Ignore(c => c.CreatedOn);
cfg.CreateMap<Address, AddressVm>();
cfg.CreateMap<AddressVm, Address>()
  .Ignore(a => a.Business)
  .Ignore(a => a.CreatedOn);

Finally, here's my SupplierController Edit Method:

[HttpPost]
public ActionResult Edit(SupplierVm supplier)
{
  if (!ModelState.IsValid) return View(supplier);

  _supplierService.UpdateSupplier(supplier);
  return RedirectToAction("Index");
}

And here's the UpdateSupplier Method on the SupplierService.cs:

public void UpdateSupplier(SupplierVm supplier)
{
  var updatedSupplier = _supplierRepository.Find(supplier.Id);
  Mapper.Map(supplier, updatedSupplier); // I lose navigational property here
  _supplierRepository.Update(updatedSupplier);
  _supplierRepository.Save();
}

I've done a load of reading and according to this blog post, what I have should work! I've also read stuff like this but I thought I'd check with readers before ditching AutoMapper for Updating Entities.

回答1:

The cause

The line ...

Mapper.Map(supplier, updatedSupplier);

... does a lot more than meets the eye.

1. During the mapping operation, updatedSupplier loads its collections (Addresses, etc) lazily because AutoMapper (AM) accesses them. You can verify this by monitoring SQL statements.
2. AM replaces these loaded collections by the collections it maps from the view model. This happens despite the UseDestinationValue setting. (Personally, I think this setting is incomprehensible.)

This replacement has some unexpected consequences:

3. It leaves the original items in the collections attached to the context, but no longer in scope of the method you're in. The items are still in the Local collections (like context.Addresses.Local) but now deprived of their parent, because EF has executed relationship fixup. Their state is Modified.
4. It attaches the items from the view model to the context in an Added state. After all, they're new to the context. If at this point you'd expect 1 Address in context.Addresses.Local, you'd see 2. But you only see the added items in the debugger.

It's these parent-less 'Modified` items that cause the exception. And if it didn't, the next surprise would have been that you add new items to the database while you only expected updates.

OK, now what?

So how do you fix this?

A. I tried to replay your scenario as closely as possible. For me, one possible fix consisted of two modifications:

1. Disable lazy loading. I don't know how you would arrange this with your repositories, but somewhere there should be a line like

   context.Configuration.LazyLoadingEnabled = false;
   

   Doing this, you'll only have the Added items, not the hidden Modified items.

2. Mark the Added items as Modified. Again, "somewhere", put lines like

   foreach (var addr in updatedSupplier.Addresses)
   {
       context.Entry(addr).State = System.Data.Entity.EntityState.Modified;
   }
   

   ... and so on.

B. Another option is to map the view model to new entity objects ...

  var updatedSupplier = Mapper.Map<Supplier>(supplier);

... and mark it, and all of its children, as Modified. This is quite "expensive" in terms of updates though, see the next point.

C. A better fix in my opinion is to take AM out of the equation completely and paint the state manually. I'm always wary of using AM for complex mapping scenarios. First, because the mapping itself is defined a long way away from the code where it's used, making code difficult to inspect. But mainly because it brings its own ways of doing things. It's not always clear how it interacts with other delicate operations --like change tracking.

Painting the state is a painstaking procedure. The basis could be a statement like ...

context.Entry(updatedSupplier).CurrentValues.SetValues(supplier);

... which copies supplier's scalar properties to updatedSupplier if their names match. Or you could use AM (after all) to map individual view models to their entity counterparts, but ignoring the navigation properties.

Option C gives you fine-grained control over what gets updated, as you originally intended, instead of the sweeping update of option B. When in doubt, this may help you decide which option to use.

回答2:

I've gotten this issue many times and is normally this:

The FK Id on the parent reference doesn't match the PK on that FK entity. i.e. If you have an Order table and a OrderStatus table. When you load both into entities, Order has OrderStatusId = 1 and the OrderStatus.Id = 1. If you change OrderStatusId = 2 but do not update OrderStatus.Id to 2, then you'll get this error. To fix it, you either need to load the Id of 2 and update the reference entity or just set the OrderStatus reference entity on Order to null before saving.

回答3:

I am not sure if this is going to fit your requirement but I would suggest following.

From your code it surely looks like you are loosing relationship during mapping somewhere.

To me it looks like that as part of UpdateSupplier operation you are not actually updating any of the child details of the supplier.

If that is the case I would suggest to updadate only changed properties from the SupplierVm to the domain Supplier class. You can write a separate method where you will assign property values from SupplierVm to the Supplier object (This should change only non-child properties such as Name, Description, Website, Phone etc.).

And then perform db Update. This will save you from possible messup of the tracked entities.

If you are changing the child entities of supplier, I would suggest to update them independent of suppliers because retrieving an entire object graph from database would require lot of queries to be executed and updating it will also execute unnecessary update queries on database.

Updating entities independently would save lot of db operations and would add to the performance of the application.

You can still use the retrieval of entire object graph if you have to display all the details about the supplier in one screen. For updates I would not recommend update of entire object graph.

I hope this would help resolving your issue.