The relationship could not be changed because one or more of the foreign-key properties is non-nullable

Question

I am getting this error when I GetById() on an entity and then set the collection of child entities to my new list which comes from the MVC view.

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 don't quite understand this line:

The relationship could not be changed because one or more of the foreign-key properties is non-nullable.

Why would I change the relationship between 2 entities? It should remain the same throughout the lifetime of the whole application.

The code the exception occurs on is simple assigning modified child classes in a collection to the existing parent class. This would hopefully cater for removal of child classes, addition of new ones and modifications. I would have thought Entity Framework handles this.

The lines of code can be distilled to:

var thisParent = _repo.GetById(1);
thisParent.ChildItems = modifiedParent.ChildItems();
_repo.Save();

Solution 1

You should delete old child items thisParent.ChildItems one by one manually. Entity Framework doesn't do that for you. It finally cannot decide what you want to do with the old child items - if you want to throw them away or if you want to keep and assign them to other parent entities. You must tell Entity Framework your decision. But one of these two decisions you HAVE to make since the child entities cannot live alone without a reference to any parent in the database (due to the foreign key constraint). That's basically what the exception says.

Edit

What I would do if child items could be added, updated and deleted:

public void UpdateEntity(ParentItem parent)
{
    // Load original parent including the child item collection
    var originalParent = _dbContext.ParentItems
        .Where(p => p.ID == parent.ID)
        .Include(p => p.ChildItems)
        .SingleOrDefault();
    // We assume that the parent is still in the DB and don't check for null

    // Update scalar properties of parent,
    // can be omitted if we don't expect changes of the scalar properties
    var parentEntry = _dbContext.Entry(originalParent);
    parentEntry.CurrentValues.SetValues(parent);

    foreach (var childItem in parent.ChildItems)
    {
        var originalChildItem = originalParent.ChildItems
            .Where(c => c.ID == childItem.ID && c.ID != 0)
            .SingleOrDefault();
        // Is original child item with same ID in DB?
        if (originalChildItem != null)
        {
            // Yes -> Update scalar properties of child item
            var childEntry = _dbContext.Entry(originalChildItem);
            childEntry.CurrentValues.SetValues(childItem);
        }
        else
        {
            // No -> It's a new child item -> Insert
            childItem.ID = 0;
            originalParent.ChildItems.Add(childItem);
        }
    }

    // Don't consider the child items we have just added above.
    // (We need to make a copy of the list by using .ToList() because
    // _dbContext.ChildItems.Remove in this loop does not only delete
    // from the context but also from the child collection. Without making
    // the copy we would modify the collection we are just interating
    // through - which is forbidden and would lead to an exception.)
    foreach (var originalChildItem in
                 originalParent.ChildItems.Where(c => c.ID != 0).ToList())
    {
        // Are there child items in the DB which are NOT in the
        // new child item collection anymore?
        if (!parent.ChildItems.Any(c => c.ID == originalChildItem.ID))
            // Yes -> It's a deleted child item -> Delete
            _dbContext.ChildItems.Remove(originalChildItem);
    }

    _dbContext.SaveChanges();
}

Note: This is not tested. It's assuming that the child item collection is of type ICollection. (I usually have IList and then the code looks a bit different.) I've also stripped away all repository abstractions to keep it simple.

I don't know if that is a good solution, but I believe that some kind of hard work along these lines must be done to take care of all kinds of changes in the navigation collection. I would also be happy to see an easier way of doing it.

Solution 2

The reason you're facing this is due to the difference between composition and aggregation.

In composition, the child object is created when the parent is created and is destroyed when its parent is destroyed. So its lifetime is controlled by its parent. e.g. A blog post and its comments. If a post is deleted, its comments should be deleted. It doesn't make sense to have comments for a post that doesn't exist. Same for orders and order items.

In aggregation, the child object can exist irrespective of its parent. If the parent is destroyed, the child object can still exist, as it may be added to a different parent later. e.g.: the relationship between a playlist and the songs in that playlist. If the playlist is deleted, the songs shouldn't be deleted. They may be added to a different playlist.

The way Entity Framework differentiates aggregation and composition relationships is as follows:

• For composition: it expects the child object to a have a composite primary key (ParentID, ChildID). This is by design as the IDs of the children should be within the scope of their parents.

• For aggregation: it expects the foreign key property in the child object to be nullable.

So, the reason you're having this issue is because of how you've set your primary key in your child table. It should be composite, but it's not. So, Entity Framework sees this association as aggregation, which means, when you remove or clear the child objects, it's not going to delete the child records. It'll simply remove the association and sets the corresponding foreign key column to NULL (so those child records can later be associated with a different parent). Since your column does not allow NULL, you get the exception you mentioned.

Solutions:

1- If you have a strong reason for not wanting to use a composite key, you need to delete the child objects explicitly. And this can be done simpler than the solutions suggested earlier:

context.Children.RemoveRange(parent.Children);

2- Otherwise, by setting the proper primary key on your child table, your code will look more meaningful:

parent.Children.Clear();

Solution 3

This is a very big problem. What actually happens in your code is this:

• You load Parent from the database and get an attached entity
• You replace its child collection with new collection of detached children
• You save changes but during this operation all children are considered as added becasue EF didn't know about them till this time. So EF tries to set null to foreign key of old children and insert all new children => duplicate rows.

Now the solution really depends on what you want to do and how would you like to do it?

If you are using ASP.NET MVC you can try to use UpdateModel or TryUpdateModel.

If you want just update existing children manually, you can simply do something like:

foreach (var child in modifiedParent.ChildItems)
{
    context.Childs.Attach(child); 
    context.Entry(child).State = EntityState.Modified;
}

context.SaveChanges();

Attaching is actually not needed (setting the state to Modified will also attach the entity) but I like it because it makes the process more obvious.

If you want to modify existing, delete existing and insert new childs you must do something like:

var parent = context.Parents.GetById(1); // Make sure that childs are loaded as well
foreach(var child in modifiedParent.ChildItems)
{
    var attachedChild = FindChild(parent, child.Id);
    if (attachedChild != null)
    {
        // Existing child - apply new values
        context.Entry(attachedChild).CurrentValues.SetValues(child);
    }
    else
    {
        // New child
        // Don't insert original object. It will attach whole detached graph
        parent.ChildItems.Add(child.Clone());
    }
}

// Now you must delete all entities present in parent.ChildItems but missing
// in modifiedParent.ChildItems
// ToList should make copy of the collection because we can't modify collection
// iterated by foreach
foreach(var child in parent.ChildItems.ToList())
{
    var detachedChild = FindChild(modifiedParent, child.Id);
    if (detachedChild == null)
    {
        parent.ChildItems.Remove(child);
        context.Childs.Remove(child); 
    }
}

context.SaveChanges();

Solution 4

I found this answer much more helpful for the same error. It seems that EF does not like it when you Remove, it prefers Delete.

You can delete a collection of records attached to a record like this.

order.OrderDetails.ToList().ForEach(s => db.Entry(s).State = EntityState.Deleted);

In the example, all of the Detail records attached to an Order have their State set to Delete. (In preparation to Add back updated Details, as part of an Order update)

Solution 5

I've no idea why the other two answers are so popular!

I believe you were right in assuming the ORM framework should handle it - after all, that is what it promises to deliver. Otherwise your domain model gets corrupted by persistence concerns. NHibernate manages this happily if you setup the cascade settings correctly. In Entity Framework it is also possible, they just expect you to follow better standards when setting up your database model, especially when they have to infer what cascading should be done:

You have to define the parent - child relationship correctly by using an "identifying relationship".

If you do this, Entity Framework knows the child object is identified by the parent, and therefore it must be a "cascade-delete-orphans" situation.

Other than the above, you might need to (from NHibernate experience)

thisParent.ChildItems.Clear();
thisParent.ChildItems.AddRange(modifiedParent.ChildItems);

instead of replacing the list entirely.

UPDATE

@Slauma's comment reminded me that detached entities are another part of the overall problem. To solve that, you can take the approach of using a custom model binder that constructs your models by attempting to load it from the context. This blog post shows an example of what I mean.

View more solutions

Comments

[yougotiger]

I found my answer buy using solution #2 in the below article, basically I created added a primary key to the child table for the reference to the parent table (so it has 2 primary keys (the foreign key for the parent table and the ID for the child table). c-sharpcorner.com/UploadFile/ff2f08/…

[antonio]

@jaffa, I found my answer here stackoverflow.com/questions/22858491/…

[redwards510]

For me the fix was simple. My db foreign key column is a nullable int, but my EF property was an int. I made it an int? to match the db and problem solved.

[jaffa]

So what if some are only changed? Does that mean I still have to remove them and add them again?

[Slauma]

@Jon: No, you can also update existing items of course. I've added an example how I would probably update the child collection, see Edit section above.

[Ladislav Mrnka]

@Slauma: Lol, if I knew that you are going to modify your answer I would not write my answer ...

[Slauma]

@Ladislav: No, no, I am glad that you wrote your own answer. Now at least I know that it's not complete nonsense and much too complicated what I did above.

[Slauma]

But there is your interesting remark about using .Clone(). Do you have the case in mind that a ChildItem has other sub-child navigation properties? But in that case, wouldn't we want that the whole sub-graph is attached to the context since we would expect that all the sub-childs are new objects if the child itself is new? (Well, might be different from model to model, but let's assume the case that the sub-childs are "dependent" from the child like the childs are dependent from the parent.)

[Ladislav Mrnka]

It would probably require "smart" clone.

[jaffa]

@Slauma: I've tried out the above code and it looks good, however I run into a problem in the removing items in the collection. The error is: "Collection was modified; enumeration operation may not execute". Any ideas how to get around this?

[Slauma]

@Jon: Try to append a ToList() after the Where method: ....Where(c => c.ID != 0).ToList().

[Slauma]

@Jon: I've changed to code to add ToList() and wrote a comment into the sample. It's indeed necessary to make a copy with ToList() because removing from the context removes also from the collection internally.

[jaffa]

@Slauma: Yes I realised that Remove() modifies the existing enumeration hence either using the ToList() in the for loop or using a separate 'var y = x.toList() then using y in the for()'. Thanks again!

[Scott Mitchell]

Because you are doing nested loops the performance on this could become an issue if the set of data to update is sufficiently large. An alternative approach is to sort the data by the key and then loop through the two sets of data one item at a time to determine whether you need to add, update, or delete the item.

[SOfanatic]

would it be easier to delete all child items and then recreate them?, or would this be slower than the nested loops?

[Slauma]

@SOfanatic: In cases where you have other entities referencing a child entity it won't be possible to delete it.

[SOfanatic]

@Slauma: yes I meant in the case where there is a one-to-many relationship between the parent and the children, so if the parent is deleted, all children should be deleted too.

[Slauma]

Setup as identifying relationship won't help here because the scenario in the question has to deal with detached entities ("my new list which comes from the MVC view"). You still have to load the original children from the DB, find the removed items in that collection based on the detached collection and then remove from the DB. The only difference is that with an identifying relationship you can call parent.ChildItems.Remove instead of _dbContext.ChildItems.Remove. There is still (EF <= 6) no built-in support from EF to avoid lengthy code like the one in the other answers.

[Andre Luus]

I understand your point. However, I believe with a custom model binder that loads the entity from the context or returns a new instance the approach above would work. I'll update my answer to suggest that solution.

[Slauma]

Yes, you could use a model binder but you had to do the stuff from the other answers in the model binder now. It just moves the problem from repo/service layer to the model binder. At least, I don't see a real simplification.

[Andre Luus]

The simplification is automatic deletion of orphaned entities. All you need in the model binder is a generic equivalent of return context.Items.Find(id) ?? new Item()

[Kirsten]

What if you don't want to have a Child's collection in your context? http://stackoverflow.com/questions/20233994/do-i-need-to-cre‌​ate-a-dbset-for-ever‌​y-table-so-that-i-ca‌​n-persist-child-enti‌​tie

[perfect_element]

I would add a condition when retrieving the originalChildItem in the foreach: ...Where(c => c.ID == childItem.ID && c.ID != 0) otherwise it will return the newly added children if the childItem.ID == 0.

[Slauma]

@perfect_element: You are absolutely right. That was an almost 3 years old serious bug in the code. Thanks!

[Pepito Fernandez]

Does the reinsertion happens with new IDs or it keeps the child's IDs they had in the first place?

[Chris Moschini]

Good feedback for the EF team, but your proposed solution doesn't solve anything in EF land unfortunately.

[Andre Luus]

How so? I implemented this and is being used as is in production software.

[Booji Boy]

I found this explanation most helpful.

[Andy Edinborough]

This worked well for me. I just needed to add context.DetectChanges();.

[JsonStatham]

Trying to get this working myself wasted over a week, thanks for posting. EF need to try and handle this themselves.

[Sampath]

@Slauma Do you know how can we do this by using repositories ? That means without using _dbContext directly.

[Chrysalis]

Good explanation for composition vs aggregation and how entity framework is relates to it.

[ryanulit]

#1 was the least amount of code necessary to fix the issue. Thank you!

[Dandy]

What is the WillCascadeOnDelete() option for then?

[Aamol]

I faced same problem and same question in my mind as like @jaffa. But from first paragraph of this answer it's more clear what EF trying to say. Thanks for explanation and answer !

[J86]

I'm facing a similar issue with AutoMapper, but this doesn't work for me :( See stackoverflow.com/q/41430679/613605

[Fernando Torres]

parent.ChildItems.Remove(child); context.Childs.Remove(child); This double remove fixed may issue, THANKS. Why do we need both removes? Why removeing just from parent.ChildItems is not enougth since childs only lives as childs?

[desmati]

I believe it's the proper answer.

[sairfan]

logical and straightforward solution.

[Bluemoon74]

You could also make a extention class for your context with these functions:

[Dalbir Singh]

+1 Great answer, I ran into this problem today and could not figure it out. Followed your solution (making ID and Foreign Key column a composite PK and my .Clear() operation finally worked. Thanks.

[Alexander Brattsev]

Thanks! I suffered for 3 hours. This is the shortest solution

[Mohammad Reza]

Actually sometimes using composite key bring complexity into the program and it is better to only have one identity column. medium.com/@pablodalloglio/…

[Sedat Kumcu]

Thanks for this useful code. my problem solved .

[Philip Stratford]

This seems to be exactly the same problem as I have. My issue with the solution is that from a DB design perspective, the composite key isn't quite right. If I'm going to add the equivalent of your ParentId column to the PK, I'll also need to add a UNIQUE constraint on the other column to ensure that it remains unique and data integrity is maintained. At the moment, the PK constraint is doing this.

[KADEM Mohammed]

Ohhh my God !!! that's Mosh Hamedani !!! :D