Is doing Transaction Management in the Controller bad practice?

Question

I'm working on a PHP/MySQL app using the Yii framework.

I've come across the following situation:

In my VideoController, I have a actionCreate which creates a new Video and actionPrivacy which sets the privacy on the Video. The problem is that during the actionCreate the setPrivacy method of the Video model is called which currently has a transaction. I would like the creation of the Video to be in a transaction as well which leads to an error since a transaction is already active.

In the comment on this answer, Bill Karwin writes

So there's no need to make Domain Model classes or DAO classes manage transactions -- just do it at the Controller level

and in this answer:

Since you're using PHP, the scope of your transactions is at most a single request. So you should just use container-managed transactions, not service-layer transa. That is, start the transaction at the start of handling the request, and commit (or rollback) as you finish handling the request.

If I manage the transactions in the controller, I would have a bunch of code that looks like:

public function actionCreate() {   $trans = Yii::app()->getDb()->beginTransaction();   ...action code...   $trans->commit(); } 

That leads to duplicated code in a lot of places where I need transactions for the action.

Or I could refactor it into the beforeAction() and afterAction() methods of the parent Controller class which would then automatically create transactions for each action being performed.

Would there be any problems with this method? What is a good practice for transaction management for a PHP app?

Answer 1

The reason that I say transactions don't belong in the model layer is basically this:

Models can call methods in other models.

If a model tries to start a transaction, but it has no knowledge of whether its caller started a transaction already, then the model has to conditionally start a transaction, as shown in the code example in @Bubba's answer. The methods of the model have to accept a flag so that the caller can tell it whether it is permitted to start its own transaction or not. Or else the model has to have the ability to query its caller's "in a transaction" state.

public function setPrivacy($privacy, $caller){     if (! $caller->isInTransaction() ) $this->beginTransaction();      $this->privacy = $privacy;     // ...action code..      if (! $caller->isInTransaction() ) $this->commit(); } 

What if the caller isn't an object? In PHP, it could be a static method or simply non-object-oriented code. This gets very messy, and leads to a lot of repeated code in models.

It's also an example of Control Coupling, which is considered bad because the caller has to know something about the internal workings of the called object. For example, some of the methods of your Model may have a $transactional parameter, but other methods may not have that parameter. How is the caller supposed to know when the parameter matters?

// I need to override method's attempt to commit $video->setPrivacy($privacy, false);    // But I have no idea if this method might attempt to commit $video->setFormat($format);  

The other solution I have seen suggested (or even implemented in some frameworks like Propel) is to make beginTransaction() and commit() no-ops when the DBAL knows it's already in a transaction. But this can lead to anomalies if your model tries to commit and finds that its doesn't really commit. Or tries to rollback and has that request ignored. I've written about these anomalies before.

The compromise I have suggested is that Models don't know about transactions. The model doesn't know if its request to setPrivacy() is something it should commit immediately or is it part of a larger picture, a more complex series of changes that involve multiple Models and should only be committed if all these changes succeed. That's the point of transactions.

So if Models don't know whether they can or should begin and commit their own transaction, then who does? GRASP includes a Controller pattern which is a non-UI class for a use case, and it is assigned the responsibility to create and control all the pieces to accomplish that use case. Controllers know about transactions because that's the place all the information is accessible about whether the complete use case is complex, and requires multiple changes to be done in Models, within one transaction (or perhaps within several transactions).

The example I have written about before, that is to start a transaction in the beforeAction() method of an MVC Controller and commit it in the afterAction() method, is a simplification. The Controller should be free to start and commit as many transactions as it logically requires to complete the current action. Or sometimes the Controller could refrain from explicit transaction control, and allow the Models to autocommit each change.

But the point is that the information about what tranasction(s) are necessary is something that the Models don't know -- they have to be told (in the form of a $transactional parameter) or else query it from their caller, which would have to delegate the question all the way up to the Controller's action anyway.

You may also create a Service Layer of classes that each know how to execute such complex use cases, and whether to enclose all the changes in a single transaction. That way you avoid a lot of repeated code. But it's not common for PHP apps to include a distinct Service Layer; the Controller's action is usually coincident with a Service Layer.