EF Deleted Items Issue

I noticed an issue in one of my service methods whereby a record I deleted showed up in a subsequent query within a single unit of work.

The example code is

int orderId = order.OrderID;
_orderRepository.delete(order);
Order newOrder = _orderRepository.getAll(x=>x.orderID == orderId);

The above example is a bit contrived, there's a fair bit more that goes on but this code highlights the issue.

Now that I know what is going on this is realtively straight forward, but it is a bit counterintuitive when starting out.

The problem was in my repository, where I was using the DbContext DbSet property for each entity directly, instead of the DbSet.Local property. The difference between the two is that the root DbSet property contains all elements in their modified state (e.g. it contains the deleted order, with an updated state of Deleted), while the Local DbSet property (which is an IObservable of the root DbSet) has the entities in their 'current state' so if you delete an entity from the context it is removed from the Local DbSet.

I say this is counterintuitive because the only way to identify whether an item is deleted or not is through the root context Entry() method, you cannot base a query on the DbSet to exclude deleted items.

The solution is however fairly simple. Since I am using a Unit of Work pattern on the context, and my service methods are a single unit of work, I can use the Local DbSet for my repository actions without any issues down the line with disconnected or orphan entities, and I can do this without any modifications to my service.

So where all my repository queries used to use the code below as the base for all repository queries
IQueryable query = _set.AsQueryable(); //_set is the appropriate DbSet for the entity T in the context
I now simply base all my queries off
IQueryable query = _set.Local.AsQueryable();

Now deleted items should not show up in my list of queries. I hope - I haven't had a chance to actually test it just yet.

*edit*

Well, that was short lived - it seems as though using the Local context only works on previously loaded data, for instance you do a load, then delete an entity, then a load from the local context will not show the deleted item.

This is incredible frustrating as it means I need to know under what scenario I am 'loading' data in order to choose the right context to load from, and means I need to front-load all the entities I will be working with, then use the local context from that point on.

I am seriously thinking of switching to nHibernate over this one.

*edit 2*

I have identified a possible solution, but I am concerned by performance implications

When performing a query on my context, I can use Linq to query the state of each entity in the resulting query and further filter the results.

query.Where(whereClause).ToList().Where(x=> ((DbContext)_context).Entry(x).State != System.Data.EntityState.Deleted ).ToList();

The two performance issues with this are

a) I need to 'ToList()' the query and then apply the state filter (otherwise EF will attempt to apply the filter to the SQL query, which it can't do). This is not ideal, but not critical, and I may be able to force the first Where() to resolve the EF part in another way to avoid the extra list creation.

and b) queries that return a large number of results will be impacted (potentially severely) since each entity will be inspected individually against the context change manager.

So perhaps an nHibernate implementation could wait if this does what I want it to without critical performance implications.

The difference between bad code and good code

A colleague asked for some advice today on a project that he inherited (which I am extending with a separate module incidentally).

The issue was related to the usage of Entity Framework in the code that he had to maintain, and he needed some advice on how to proceed.  The problem was that the service layer was calling the repository multiple times, but each repository method was wrapped in a separate unit of work.
e.g.

public void DeleteEntity(int entityID)
{
    using (var context = EntityContext())
    {
        var entity = context.Postings.SingleOrDefault(p => p.entityID == entityID);
        context.Entities.DeleteObject(entity);
        context.SaveChanges();
    }
}
and
public Entity GetPosting(int entityID)

{
    using (var context = EntityContext())
    {
        return context.Entities.FirstOrDefault(p => p.entityID == entityID);
    }
}

This caused two problems for the developer, who needed to perform a complex action in his service that referenced multiple repository calls.

1. He had no control over the transactional scope for the repository methods
2. Each operation was on a separate EF context, so the service could not load and entity, edit it, and then save the changes (unless the repository was designed for disconnected entities, which it wasn't).

From a maintainability and testability point of view this was also a very poor design, as the repository methods created instances of dependency object (the service method also created instances of the repositories, making the services inherently untestable).


The version of this design that I implemented for my component follows a similar service/repository/entity pattern, but is implemented in a far more testable and robust manner.

The first improvement over the legacy design is in the dependency management
My service accepts a context and all required repositories in the constructor, and my repositories accepts a context, which allows for improved maintainability (all dependencies are described) and testability (all dependencies can be mocked).  This also allows us to use dependency injection/IoC to create our object instances.

The second improvement was in the Unit of Work design.
Rather than have each repository method as a single unit of work, the service methods are the units of work, so any action within the service uses the same context (as it is passed as a dependency to the repositories that the service uses), and each service call acts as a Unit of Work, calling SaveChanges at the end of the service to ensure that the changes act under a single transaction.
There are limitations to this design (your public service methods become an atomic transaction and you should not call other public methods from within another method) but for simplicity and maintainability it is a pretty good solution.

Below is a simple example of the design I am using, preserving maintainability, testability, and predictability.  I'm not saying it is necessarily the best code around, but it solves a number of issues that I often see in other developers code.

public class HydrantService
{
  public HydrantService(HydrantsSqlServer context, EFRepository<Hydrant> hydrantRepository, EFRepository<WorkOrder> workOrderRepository, EFRepository<HydrantStatus> hydrantStatusRepository)
  {
    _context = context;
    _hydrantRepository = hydrantRepository;
    _workOrderRepository = workOrderRepository;
    _hydrantStatusRepository = hydrantStatusRepository;
  }
  public void createFaultRecord(WorkOrder order)
  {
    HydrantStatus status = _hydrantStatusRepository.GetSingle<HydrantStatus>(x => x.StatusCode == "Fault"); //_context.HydrantStatuses.Where(x => x.StatusCode == "Fault").FirstOrDefault();
    order.Hydrant.HydrantStatus = status;
    _workOrderRepository.Add(order);
    _context.SaveChanges();
  }
}

  public class EFRepository<T>
 {
 public EFRepository(IDbContext context)
 {
    _context = context;
  }   public virtual ICollection GetAll()

  {
    IQueryable query = _context.Set();
    return query.ToList();
  }
}