Using DbContext SaveChanges with Transactions

Question

As MSDN confirms, in EF 5 and on, the DbContext class is "a combination of the Unit-Of-Work and Repository patterns." In the web applications I build, I tend to implement the Repository and Unit-Of-Work patterns on top of the existing DbContext class. Lately, like many others out there, I've found that this is overkill in my scenario. I am not worried about the underlying storage mechanism ever changing from SQL Server, and while I appreciate the benefits that unit testing would bring, I still have a lot to learn about it before actually implementing it in a live application.

Thus, my solution is to use the DbContext class directly as the Repository and Unit-Of-Work, and then use StructureMap to inject one instance per request to individual service classes, allowing them to do work on the context. Then in my controllers, I inject each service I need and call the methods necessary by each action accordingly. Also, each request is wrapped in a transaction created off of the DbContext at the beginning of the request and either rolled back if any type of exception occurred (whether it be an EF error or application error) or committed if all is well. A sample code scenario is below.

This sample uses the Territory and Shipper tables from the Northwind sample database. In this sample admin controller, a territory and a shipper are being added at the same time.

Controller

public class AdminController : Controller 
{
    private readonly TerritoryService _territoryService;
    private readonly ShipperService _shipperService;

    public AdminController(TerritoryService territoryService, ShipperService shipperService)
    {
        _territoryService = territoryService;
        _shipperService = shipperService;
    }

    // all other actions omitted...

    [HttpPost]
    public ActionResult Insert(AdminInsertViewModel viewModel)
    {
        if (!ModelState.IsValid)
            return View(viewModel);

        var newTerritory = // omitted code to map from viewModel
        var newShipper = // omitted code to map from viewModel

        _territoryService.Insert(newTerritory);
        _shipperService.Insert(newShipper);

        return RedirectToAction("SomeAction");
    }
}

Territory Service

public class TerritoryService
{
    private readonly NorthwindDbContext _dbContext;

    public TerritoryService(NorthwindDbContext dbContext) 
    {
        _dbContext = dbContext;
    }

    public void Insert(Territory territory)
    {
        _dbContext.Territories.Add(territory);
    }
}

Shipper Service

public class ShipperService
{
    private readonly NorthwindDbContext _dbContext;

    public ShipperService(NorthwindDbContext dbContext) 
    {
        _dbContext = dbContext;
    }

    public void Insert(Shipper shipper)
    {
        _dbContext.Shippers.Add(shipper);
    }
}

Creation of Transaction on Application_BeginRequest()

// _dbContext is an injected instance per request just like in services
HttpContext.Items["_Transaction"] = _dbContext.Database.BeginTransaction(System.Data.IsolationLevel.ReadCommitted);

Rollback or Commit of Transaction on Application_EndRequest

var transaction = (DbContextTransaction)HttpContext.Items["_Transaction"];

if (HttpContext.Items["_Error"] != null) // populated on Application_Error() in global
{
    transaction.Rollback();
}
else
{
    transaction.Commit();
}

Now this all seems to work well, but the only question I have now is where is it best to call the SaveChanges() function on the DbContext? Should I call it in each Service layer method?

public class TerritoryService
{
    // omitted code minus changes to Insert() method below

    public void Insert(Territory territory)
    {
        _dbContext.Territories.Add(territory);
        _dbContext.SaveChanges();  // <== Call it here?
    }
}

public class ShipperService
{
    // omitted code minus changes to Insert() method below

    public void Insert(Shipper shipper)
    {
        _dbContext.Shippers.Add(shipper);
        _dbContext.SaveChanges();  // <== Call it here?
    }
}

Or should I leave the service class Insert() methods as is and just call SaveChanges() right before the transaction is committed?

var transaction = (DbContextTransaction)HttpContext.Items["_Transaction"];

// HttpContext.Items["_Error"] populated on Application_Error() in global
if (HttpContext.Items["_Error"] != null) 
{
    transaction.Rollback();
}
else
{
    // _dbContext is an injected instance per request just like in services
    _dbContext.SaveChanges(); // <== Call it here?
    transaction.Commit();
}

Is either way okay? Is it safe to call SaveChanges() more than once since it is wrapped in a transaction? Are there any issues I may run into by doing so? Or is it best to call SaveChanges() just once right before the transaction is actually committed? I personally would rather just call it at the end right before the transaction is committed, but I want to be sure I am not missing any gotcha's with transactions or doing something wrong? If you read this far, thanks for taking the time to help. I know this was a long question.

Answer 1

You would call SaveChanges() when it's time to commit a single, atomic persistence operation. Since your services don't really know about each other or depend on each other, internally they have no way to guarantee one or the other is going to commit the changes. So in this setup I imagine they would each have to commit their changes.

This of course leads to the problem that these operations might not be individually atomic. Consider this scenario:

_territoryService.Insert(newTerritory);  // success
_shipperService.Insert(newShipper);  // error

In this case you've partially committed the data, leaving the system in a bit of an unknown state.

Which object in this scenario is in control over the atomicity of the operation? In web applications I think that's usually the controller. The operation, after all, is the request made by the user. In most scenarios (there are exceptions, of course) I imagine one would expect the entire request to succeed or fail.

If this is the case and your atomicity belongs at the request level then what I would recommend is getting the DbContext from the IoC container at the controller level and passing it to the services. (They already require it on their constructors, so not a big change there.) Those services can operate on the context, but never commit the context. The consuming code (the controller) can then commit it (or roll it back, or abandon it, etc.) once all of the services have completed their operations.

While different business objects, services, etc. should each internally maintain their own logic, I find that usually the objects which own the atomicity of operations are at the application level, governed by the business processes being invoked by the users.

Answer 2

You're basically creating a repository here, rather than a service.

To answer your question you could just ask yourself another question. "How will I be using this functionality?"

You're adding a couple of records, removing some records, updating some records. We could say that you're calling your various methods about 30 times. If you call SaveChanges 30 times you're making 30 round-trips to the database, causing a lot of traffic and overhead which COULD be avoided.

I usually recommend doing as few database round-trips as possible, and limit the amount of calls to SaveChanges(). Therefore I recommend that you add a Save() method to your repository/service layer and call it in the layer which calls your repository/service layer.

Unless it is absolutely required to save something before doing something else you shouldn't call it 30 times. You should call it 1 single time. If it is necessary to save something before doing something else you could still call SaveChanges in that absolute moment of requirement in the layer calling your repository/service layer.

Summary/TL;DR: Make a Save() method in your repository/service layer instead of calling SaveChanges() in each repository/service method. This will boost your performance and spare you the unnecessary overhead.