How can I best write a model that wraps a task in a transaction if and only if the calling code has not already begun a transaction?
The problem is illustrated with this example:
<?php
class TheController extends CController {
public function actionOne() {
Thing::doSomething();
}
public function actionTwo() {
// this action method implements a task
// that involves a sequence operations that includes:
Thing::doSomething();
// as well as other database operations
// and the whole task needs to be
// wrapped in a transaction
}
}
class Thing extends CActiveRecord {
public static function doSomething() {
// this method implements a task
// that involves a sequence of database operations
// that need to be wrapped in a transaction
}
}
TheController::actionOne() calls Thing::doSomething(). In this case Thing::doSomething() should employ a transaction (using the conventional Yii beginTransaction try/catch commit/rollback pattern).
TheController::actionTwo()'s task has more work, part of which is done by calling Thing::doSomething(). So TheController::actionTwo() needs to begin a transaction itself and wrap its whole task up in the try/catch. In this case Thing::doSomething() must not employ a transaction. It must detect that its caller already has an active transaction and not disrupt it.
One solution is to put code that conditionally employs a transaction in Thing::doSomething(). But it’s not a trivial amount of code. So if there are many model methods that need the same logic, it’s not a good solution.
Here’s what I considered a solution that’s not very satisfying. That’s a lot of stuff to cut and paste into every model method that needs this behavior.
public static function doSomething() {
$curTr = Yii::app()->db->getCurrentTransaction();
$transaction = $curTr === null || !$curTr->getActive()
? app()->db->beginTransaction()
: false;
try {
// sequence of database operations that need to be wrapped in a transaction
if ($transaction)
$transaction->commit();
} catch (Exception $e) {
if ($transaction) {
$transaction->rollback();
// optionally do something about the failure here, e.g.
Yii::log(__CLASS__ . '::' . __FUNCTION__ . '() failed');
return false;
} else
throw $e;
}
}
Note: I have two custom functions that handles the saving of related models. (relatedSave() and setRelatedData()) Also the tr() function is a call to the yii trace function.
I don’t show them here because its out of the scope since were talking about transactions.
<?php
class foo extends CActiveRecord
{
public function save($runValidation=true,$attributes=null)
{
if( Yii::app()->db->getCurrentTransaction() === null )
{
$transaction = Yii::app()->db->beginTransaction();
$transaction->attachbehavior('modelSaveTransaction', new CBehavior);
tr('This transaction is owned by an IcsActiveRecord->save');
}
$currentTransaction = Yii::app()->db->getCurrentTransaction();
if (!isset($currentTransaction->modelSaveTransaction))
{
tr('This save is called in an outside transaction, not owned by IcsActiveRecord->save.');
if (!isset($currentTransaction->firstSaveIsCalled))
{
$currentTransaction->attachbehavior('firstSaveIsCalled', new CBehavior);
$startedTheSave = true;
}
}
try {
parent::save($runValidation, $attributes);
if(!empty($this->relationsHolder))
$this->relatedSave($runValidation); //this function also calls foo::save()
if($this->hasErrors())
{
throw new CDbException('Model(s) do not pass validation.');
}
else
{
if(isset($transaction))
$transaction->commit();
return true;
}
}
catch (Exception $e)
{
if(isset($transaction))
$transaction->rollback();
else if (isset($startedTheSave))
throw $e;
tr('Class: '.get_class($this) .'. '.'Exception in save() catched! '.$e->getMessage());
return false;
}
}
}
####################################################
# CONTROLLER A works
public function actionUpdateA($id)
{
$model=$this->loadModel($id);
// Uncomment the following line if AJAX validation is needed
// $this->performAjaxValidation($model);
if(isset($_POST['Incident']))
{
$model->attributes=$_POST['Incident'];
$model->setRelatedData('products');
$model->setRelatedData('services');
if($model->save())
$this->redirect(array('view','id'=>$model->id));
}
$this->render('update',array(
'model'=>$model,
));
}
# CONTROLLER B works
public function actionUpdateB($id)
{
$model=$this->loadModel($id);
// Uncomment the following line if AJAX validation is needed
// $this->performAjaxValidation($model);
if(isset($_POST['Incident']))
{
$model->attributes=$_POST['Incident'];
$model->setRelatedData('products');
$model->setRelatedData('services');
$transaction = Yii::app()->db->beginTransaction();
try {
if($model->save())
$this->redirect(array('view','id'=>$model->id));
} catch (Exception $e)
{
$transaction->rollback();
tr('Catched by controller');
}
}
$this->render('update',array(
'model'=>$model,
));
}
?>
What I wanted to do is the following:
if I call save() from the controller, the save needs to internally handle its own save and also the saving of the related models. If one or more of the save()'s called internally dont succeed, it needs to return ALL validation errors. (Not just the first error it finds.)
If I start a transaction inside the controller (so out of scope of the save functions), the save function needs to rethrow the exception.
Jeroendh, thanks for your contribution. I was starting to think that nobody cares about this topic, which is an alarming thing to think.
So we discussed this at length on the #yii channel on freenode.net and I remain of the opinion that method-local state and CDbConnection instance state is sufficient. I don’t see how the need for your behavior arises.
BTW, folks: some good work gets done on #yii. Join us!
On the other hand, you point out an important feature missing from my pattern: a method should return false to indicate an error but that no rollback is required, at least as far as that method knows. For example, validation failed. This led me to the conclusion that the pattern’s method should have three ways to finish:
Return true on success
Return false on error AND the database state was not altered
Throw an exception on error AND the database state might have been altered
So right now my pattern is as follows. First we need to declare the above as a policy fo the sake of terminology.
[size="5"]Policy X[/size]
When a Policy X-conformant method returns false it is guaranteeing to its caller that neither it nor anything it called altered DB state.
When a Policy X-conformant method detects an error condition but cannot guarantee that it hasn’t altered DB state it must throw an exception.
With that in place, here’s my new template:
/**
* This method is an attempt at describing a pattern that can be used in writing
* controller and model methods in Yii that impelment Policy X and does database work
* that requires a transaction. Additionally, it assumes that the calling code may
* already have an active transaction.
*
* @return bool True on success. False on error and DB state is unchanged.
* @throws CDbException|Exception on error and the DB state may have changed.
*/
public function doSomething() {
$localTransaction =
Yii::app()->db->getCurrentTransaction() === null
? Yii::app()->db->beginTransaction()
: false;
try {
$thing = new Something; // a model instance, just for example's sake
/**
* Do any work that doesn't alter the DB state. If you do any explicit
* validations, return false on failure. Or if a call to a Policy X-conformant
* method returns false, you return false, e.g. assuming Part::stamp() is
* Policy X conformant:
*/
$part = new Part;
if (!$part->stamp()) {
$thing->addError($part->errors());
return false;
}
/**
* Up to here self::doSomething() has not altered DB state. From now on,
* assume it cannot guarantee that DB state is unaltered.
*
* Do any remaining work required. If an error condition is detected, throw
* a suitable exception, e.g.:
*/
if (!$part->stamp())
throw new CDbException('Message' /*suitable arguments*/);
if ($localTransaction)
$localTransaction->commit();
} catch (Exception $e) {
if ($localTransaction) {
$localTransaction->rollback();
return false;
} else
throw $e;
}
return true;
}
I still have no idea how to use OOP to write this template as a something that can be reused whenever a method wants to do this kind of thing.
For discussions sake, Im making a few alterations to your code to see what happens… Added doSomething’s and a CAR::Save().
/**
* This method is an attempt at describing a pattern that can be used in writing
* controller and model methods in Yii that impelment Policy X and does database work
* that requires a transaction. Additionally, it assumes that the calling code may
* already have an active transaction.
*
* @return bool True on success. False on error and DB state is unchanged.
* @throws CDbException|Exception on error and the DB state may have changed.
*/
public function doSomething() {
$localTransaction =
Yii::app()->db->getCurrentTransaction() === null
? Yii::app()->db->beginTransaction()
: false;
try {
$part = new Part;
/**
* First do any work that doesn't alter the DB state. Start by validating
* all the changes that you will make in the DB later.
*/
$this->validate();
if (!$part->doSomething()) {
$this->addError($part->errors());
}
if ($this->hasErrors()) {
return false;
}
/**
* Up to here self::doSomething() has not altered DB state. From now on,
* assume it cannot guarantee that DB state is unaltered.
*
* Do any remaining work required. If an error condition is detected, throw
* a suitable exception, e.g.:
*/
if(!$this->save())
throw new Exception('message');
if (!$part->doSomething()) //Why is this here again, if we validate above anyway??
throw new CDbException('Message' /*suitable arguments*/); //Why is this here again, if we validate above anyway??
if ($localTransaction)
$localTransaction->commit();
} catch (Exception $e) {
if ($localTransaction) {
$localTransaction->rollback();
return false;
} else
throw $e;
}
return true;
}
Yes, you’re right that addErrors() should be called on $this, not on $part.
Well, if we remove the part you dislike:
if (!$part->doSomething()) //Why is this here again, if we validate above anyway??
throw new CDbException('Message' /*suitable arguments*/); //Why is this here again, if we validate above anyway??
then we get back to the same thing that I had.
I think you are making assumptions about what doSomething() does that I was trying not to make. Apart from that I think we have arrived at the same place.
$part->doSomething() is just another example of a method that, since it obeys Policy X, will work just fine as you have shown. Whatever example makes it clear to you is fine and I think that one does so I have no objection.
