1
public void ReassignLineItems(InvoiceUpdateDto invoiceUpdate)
{
    Invoice invoiceInRepository = _unitOfWork.InvoiceRepository
        .FirstOrDefault(invoice => invoice.Id == invoiceUpdate.Id);

    invoiceInRepository.LineItems = invoiceUpdate.LineItems;
    _unitOfWork.SaveChanges();
}

After retrieving Invoice from the repository, and then detaching LineItem from a navigation collection property by either removing it from the collection, or by assigning a new collection to Invoice, Entity Framework tries to set the foreign key value to NULL. Instead, I want the foreign key to be intact, but the IsDeleted property of LineItem to change. I don't want to manually set the IsDeleted property to false, as that does not seem like a business logic responsibility.

In order to achieve this, I had to do the following.

public new void SaveChanges()
{
    var entries = ChangeTracker.Entries()
        .Where(e => e.State == EntityState.Added ||
                    e.State == EntityState.Modified ||
                    e.State == EntityState.Deleted);
foreach (var entry in entries)
{
    if (entry.Entity is LineItem lineItem)
    {
        if (entry.State == EntityState.Modified
            && lineItem.Order == null)
        {
            lineItem.IsDeleted = true;
            entry.Property(nameof(LineItem.Order)).IsModified = false;
        }
    }
}

base.SaveChanges()

}

Make a custom SaveChanges method in the DbContext class and have it find the detached LineItem from the ChangeTracker, and set the IsDeleted to false IF the foreign key property was null, and have the change to the foreign key property be ignored. Finally, call base.SaveChanges.

This works but it means that for each type that is removed from a navigation collection property, I have to write code inside the custom SaveChanges method for that type. This could make the SaveChanges method grow too large and cause performance issues because there would be a lot of if statements and type checks.

Is there a better way to achieve this?

EMN
  • 485

1 Answers1

4

I think there is a misconception here. The repository pattern is a persistence pattern. What you are describing is business logic, not persistence logic.

There is a business process you need to model, and the first step in doing this is to create a proper domain model, not an anemic domain model like you currently have (related search: anemic vs rich domain model).

The ReassignLineItems method belongs on one of the entities where they set the Order and IsDeleted properties instead of stuffing this logic in a repository. You shouldn't need to test for an entity's state in the ORM. If properly configured, your ORM should pick up the changes automatically and issue the proper UPDATE statements to the database.

This new method belongs on one of the existing entities, but this process might have a representation in both entities. A concrete recommendation relies on a myriad details which you don't have included in your question. Instead, take this as an opportunity to reconsider the design of this part of your system. You have found non-trivial business logic, and it needs a place to live. You've also discovered that mixing business logic with persistence logic can be troublesome. The solution is separation of concerns and encapsulation; two fundamental tenets of object-oriented design that an anemic domain model flippantly and maliciously throws out the window.


If I remember correctly, Entity Framework 6 is an older version, and I don't remember if it can map private properties. If it can't, I would still recommend adding proper methods to your entities so this logic has a place to live even if properties have public setters and getters. Encapsulation should be enforced by code review to ensure nobody is bypassing the new methods. Not ideal, but it's still better than what you've got now.

An alternative that I've seen many times (though it isn't my preference) is to put this logic in a service class.

This is a decision that forces you to make trade-offs due to a constraint imposed on you by a tool. A tool which may not be possible or practical to upgrade.