Web API : Centralize business transaction logic : Good Idea?

Question

We develop on a ASP.NET Web API where we use the "Unit Of Work / Repository" pattern :

Our Controllers looks like that :

public class MyController : Controller
{
 private IUnitOfWork _unitOfWork;
 private IMyService _myService;
 public MyController(IUnitOfWork unitOfWork, IMyService myService)
 {
 _unitOfWork = unitOfWork;
 _myService = myService;
 }
 [HttpPost]
 public IActionResult CreateItem(Item item)
 {
 try
 {
 _unitOfWork.BeginTransaction();
 _myService.CreateItem(item);
 _unitOfWork.Commit();
 }
 catch (Exception e)
 {
 // Transaction failed, we must rollback to keep a consistent state
 _unitOfWork.Rollback();
 // Logging
 //...
 // Return HTTP 500 Error
 return StatusCode(Microsoft.AspNetCore.Http.StatusCodes.Status500InternalServerError);
 }
 return StatusCode(Microsoft.AspNetCore.Http.StatusCodes.Status200OK);
 }
}

We will have multiple controller with the same logic :

• Start a transaction
• Do business logic
• Commit the changes
• Handle possible exceptions (Rollback, Log, return appropriate HTTP status)

So, we are wondering if we can "centralize" this code in order to avoid duplication

Only the business logic will differ

In this class :

_myService.CreateItem(item);

Is it really a good idea ?

And if yes what pattern would be the best ?

EDIT

After some more investigation it seems that the only thing we really need to centralize is the exception handling

In this case, we would have this kind of controllers :

public class MyController : Controller
{
 private IUnitOfWork _unitOfWork;
 private IMyService _myService;
 public MyController(IUnitOfWork unitOfWork, IMyService myService)
 {
 _unitOfWork = unitOfWork;
 _myService = myService;
 }
 [HttpPost]
 public void CreateItem(Item item)
 {
 using (var uow = _unitOfWork.BeginTransaction())
 {
 _myService.CreateItem(item);
 _unitOfWork.Commit();
 }
 }
}

The exception handling would be centralized with ExceptionFilter as explained here :

https://docs.microsoft.com/en-us/aspnet/web-api/overview/error-handling/web-api-global-error-handling#registering-exception-filters

For the Rollback, it seems there is no need to handle it manually :

https://stackoverflow.com/questions/27130529/dbcontexttransaction-clarification-about-rollback/27130680#27130680

Answer 1

Of course you could move this code to some class that handles this for you, for example using the business logic as an Action (https://msdn.microsoft.com/en-us/library/system.action(v=vs.110).aspx) or something like that. This could definitely be a solution for the transaction management and probably for the HTTP 500 and 200 result codes.

public class MyTransactionManager // Think of proper names
{
 public void Execute(Action action)
 {
 try
 {
 _unitOfWork.BeginTransaction();
 action();
 _unitOfWork.Commit();
 }
 catch (Exception e)
 {
 // .. handling .. 
 }
 }
}

Calling it from the Controller:

[HttpPost]
public IActionResult CreateItem(Item item)
{
 Action action = delegate() { _myService.CreateItem(item); } ;
 _transactionManager.Execute(action);
}

you may need to check the details about calling the Action, haven't tried this).

However, you will also need some error handling for specific situations. Any call to your WebAPI can have different situations when to return which HTTP error code like the 4xx's. Those checks will always be in the Controller itself.

Don't get tempted to return StatusCodes from your business logic.

• Sylvain: Thanks for the answer ! The question was maybe a naive, but I prefer to check twice before this kind of refactoring As you said, controllers needs to return the accurate status code (5XX, 4XX) depending on the exceptions But in your example MyTransactionManager catch the exceptions so that the controller have no knowledge about them Further, the controller logic about what status has to be returned can be centralized no ? (If BusinessException => 400, If InternalException => 500)

Answer 2

Is it really a good idea ?

Depends on your use case, however if you can centralise it then you have less code to maintain.

And if yes what pattern would be the best ?

You could use a an action filter, if it's used in every Api call make it a global filter.

[AttributeUsage(AttributeTargets.Method)]
public class TransactionAttribute : ActionFilterAttribute
{
 private IUnitOfWork unitOfWork;
 public TransactionAttribute()
 {
 unitOfWork = //Resolve the unit of work
 }
 public override void OnActionExecuting(ActionExecutingContext filterContext)
 {
 _unitOfWork.BeginTransaction();
 }
 public override void OnActionExecuted(ActionExecutedContext filterContext)
 {
 if (filterContext.Exception == null)
 {
 try
 {
 _unitOfWork.Commit();
 }
 catch(Exception ex)
 {
 _unitOfWork.Rollback();
 }
 }
 else
 {
 _unitOfWork.Rollback();
 }
 }
}

You can use the filter like this,

public class MyController : Controller
{
 private IMyService _myService;
 public MyController(IMyService myService)
 {
 _myService = myService;
 }
 [HttpPost]
 [Transaction]
 public IActionResult CreateItem(Item item)
 {
 _myService.CreateItem(item);
 //return the status code
 }
}

• gnat: this reads more like a comment, see How to Answer