Entity Framework proper way to replace collection in one to many

Question

Suppose a customer has many phone numbers and a phone number has only one customer.

public class PhoneNumber : IValueObject {
  public string Number {get; set;}
  public string Type {get; set;}
}

public class Customer : IEntity {
   public ICollection<PhoneNumber> phones {get; private set;} //ew at no encapsulated collection support
   public void SetPhones(params PhoneNumber[] phones) {
       this.phones.Clear();
       this.phones.AddRange(phones);
   }
}

If I do an EF mapping like this and run it, every time I set phone numbers it will create new PhoneNumbers but not delete the old ones. There are no other entities referencing phone numbers, I don't even expose it on my dbcontext, is there a way to tell EF that Customer owns PhoneNumbers completely and therefore if phone numbers were removed from the collection they should be deleted?

proof

I realize that there's a dozen ways to hack around this problem, but this isn't a weird edge case, what's the "right" way to handle this.

I think reverse is possible if you would delete phone numbers form dbset and cascasde on delete is turned on then it will result in what you wanted exactly :) — Jenish Rabadiya, May 08 '15 at 05:50
@JenishRabadiya I think that with cascade on delete I could maybe have phone numbers deleted when deleting `Customer`, but this is specifically about modification... — George Mauer, May 08 '15 at 13:35
Please post the part of the code where you try to remove a `PhoneNumber` from a `Customer`. — Alexandre Severino, May 08 '15 at 16:20
@Kilouco the `SetPhones` method is the only one so far that does anything. — George Mauer, May 08 '15 at 16:26
[This seems related](http://stackoverflow.com/questions/17850702/a-relationship-is-in-the-deleted-state) — George Mauer, May 08 '15 at 16:30

score 18 · Accepted Answer · edited May 23 '17 at 12:17

18

I had the exact same question :)

This answer on identifying relationships solved my issue.

Note: You have to load the collection (eagerly, explicitly or lazily) so that it can be tracked before setting the new values and calling save. Otherwise you will not be replacing the collection but, just be adding to it.

For example:

var entity = unitOfWork.EntityRepository.GetById(model.Id);
// I have something like this to load collection because
// I don't have the collection's entities exposed to the context
unitOfWork.EntityRepository.LoadCollection(entity, e => e.CollectionProperty);
entity.CollectionProperty = newCollectionValuesList;
unitOfWork.Save();

This will remove the previous collection values from the 'collection table' and only add the newly set values.

Hope that helps.

edited May 23 '17 at 12:17

Community

1
1

answered Jul 20 '15 at 09:11

Quinton Smith

2,550
18
17

+1 for actually doing things correctly and having repositories flow "out of" a unit of work. I still don't really get the answer though. What would `Entity` look like here? Are you saying that this trick with phone number ids *alone* is enough to get it to trigger a delete properly? – George Mauer Jul 20 '15 at 12:45
1

@GeorgeMauer - Entity here would be your `Customer` and `CollectionProperty` would be your `phones` property. In my case I'm loading my collections explicitly so `LoadCollection` looks like `context.Entry(entity).Collection(navigationProperty).Load();` under the hood. If you have an identifying relationship between Customer and PhoneNumber and you have loaded `phones` for a customer then the code in your screenshot should leave you with only 123-456-7890 in the PhoneNumbers table. – Quinton Smith Jul 20 '15 at 18:13
@QuintonSmith is that "Save" method you have written out which is really eventually just calling "SaveChanges()?" – Kieran Ojakangas Sep 05 '20 at 18:31
@KieranOjakangas you are exactly right. Under the hood it's simply `context.SaveChanges();` – Quinton Smith Sep 07 '20 at 14:50
Thank you very much @QuintonSmith, you saved my day with your comment that child collections need to be loaded to be replaced. – Rex Jun 17 '21 at 09:13

Alexandre Severino · Answer 2 · 2015-05-08T16:34:10.850

4

First (optional):

I recommend you to make

public ICollection<PhoneNumber> phones {get; private set;}

a virtual property, to let the Entity Framework know that it should be lazy loaded (even if you do not have Lazy Load enabled, it is a good practice).

public virtual ICollection<PhoneNumber> phones {get; private set;}

Second:

Add an Inverse Navigation Property on your PhoneNumber class (it will be required in order to achieve the solution I give you below):

public class PhoneNumber : IValueObject {
  public string Number {get; set;}
  public string Type {get; set;}

  public virtual Customer {get; set;}
}

public class Customer : IEntity {
   public ICollection<PhoneNumber> phones {get; private set;} //ew at no encapsulated collection support
   public void SetPhones(params PhoneNumber[] phones) {
       this.phones.Clear();
       this.phones.AddRange(phones);
   }
}

Third (Possible solution for your problem):

Remove the PhoneNumber objects from Context instead of doing so from the Customer:

public ICollection<PhoneNumber> phones {get; private set;} //ew at no encapsulated collection support
   public void SetPhones(params PhoneNumber[] phones) {
       Context.PhoneNumbers.RemoveRange(this.phones);
       this.phones.AddRange(phones);
   }
}

edited May 08 '15 at 16:34

answered May 08 '15 at 15:01

Alexandre Severino

1,563
1
16
38

Error during `Update-Database`. `An error occurred while saving entities that do not expose foreign key properties for their relationships. The EntityEntries property will return null because a single entity cannot be identified as the source of the exception. Handling of exceptions while saving can be made easier by exposing foreign key properties in your entity types...A relationship from the 'Customer_phones' AssociationSet is in the 'Deleted' state. Given multiplicity constraints, a corresponding 'Customer_phones_Target' must also in the 'Deleted' state.` – George Mauer May 08 '15 at 16:04
@GeorgeMauer try commenting the Third step and do only the first two. They should be enough to solve your problem. – Alexandre Severino May 08 '15 at 16:14
Removing the third step (even when I keep the `PhoneNumber`->`Customer` relationship) causes the issue in the screenshot above. Old phone numbers now just have a null customer, they are not removed. – George Mauer May 08 '15 at 16:35
Also, instead of doing my new "third step" you could try adding `[Required]` right above the `public virtual Customer {get; set;}` or adding the property `public int CustomerId {get; set;}` which signs EF that the Customer MUST NOT be null. – Alexandre Severino May 08 '15 at 16:38
Please let me know if any of the new ideas on my comments work so I can edit the answer. – Alexandre Severino May 08 '15 at 16:46
Have to run out now, will be back to it in a few hours – George Mauer May 08 '15 at 16:56
In your update, where does `Context` come from? Wouldn't that mean that entities have to be connected to the db context? How on earth would you uni test that? – George Mauer May 08 '15 at 21:19
Nevermind my Third answer if you need a uni test... have you tried the `[Required]` and adding the `CustomerId` property instead? – Alexandre Severino May 08 '15 at 22:22
I have but the core issue is that this doesn't tell Entity Framework that `PhoneNumber` is not used by a different class. So required or not, how would it know that it is ok to delete this? – George Mauer May 09 '15 at 22:53
EF knows that your `PhoneNumber` is not being used anywhere else than in `Costumer`. Thus, if the `Required` is set on PhoneNumber's `Customer` inverse property and this very `Customer` ceases to exist, `PhoneNumber` should be automatically cascade deleted. – Alexandre Severino May 11 '15 at 16:03

Entity Framework proper way to replace collection in one to many

2 Answers2

Linked