Db trans rollback but leave model primary key exists

[a761208]

What steps will reproduce the problem?

namespace app\commands;

use app\models\Model1;
use Yii;
use yii\base\Exception;
use yii\console\Controller;

class TestController extends Controller
{
    public function actionIndex()
    {
        $model = new Model1();
        $model->... = '...'; // fill with correct values
        try {
            Yii::$app->db->transaction(function () use (&$model) {
                $model->save();
                throw new Exception('Another error occurred.');
            });
        } catch (\Throwable $e) {
            $this->stdout($e->getMessage() . PHP_EOL);
            $this->stdout($model->id . PHP_EOL);
        }
    }
}

What is the expected result?

Empty $model->id

What do you get instead?

There is a value in $model->id

Additional info

Q	A
Yii version	2.0.14
PHP version	PHP 7.0.27-1+ubuntu16.04.1+deb.sury.org+1 (cli) (built: Jan 5 2018 14:12:46) ( NTS )
Operating system	Ubuntu 16.04.3 LTS

[SilverFire]

Woah, this one will not be an easy one :)

[SilverFire]

@a761208 in case you have an idea how to fix it – we will be happy to see a Pull Request from you

[beroso]

I was facing the same problem, but ended up setting pk manually to null. I have a idea now, I will try to submit a pull request soon.

[a761208]

@berosoboy NOT just insert and not just ActiveRecord. Every thing changed in the trans should be reset after rollback.

[fcaldarelli]

The model should be used to populate the form input controls. So, although transaction had thrown an exception, the model should contain all attributes, also that changed by request (post or get). The only thing to reset is the pk (in catch exception block), that in create action is never read from input. So even if the primary key remained set, this should not be a problem.

[beroso]

Every thing changed in the trans should be reset after rollback.

@a761208 It could lead to undesirable behavior. The programmer needs to be responsible to handle it.

I agree with you @FabrizioCaldarelli . My PR, (work in progress) targets only the pk reset and, most important, the $model->isNewRecord reset. (this last made me crazy once :D , because sometimes I trust the $model->isNewRecord in my view files)

[SamMousa]

The only thing to reset is the pk (in catch exception block), that in create action is never read from input.

That is an incorrect assumption for several reasons:

1. There might be auto increment fields that are not PKs in some DBMSes.
2. It is perfectly valid to have a PK that is not auto incremented (and which thus should not be reset).

I think this can be partially solved using the event system (ie registering the model to revert its attributes on the ROLLBACK event). However there are still scenarios where behavior will be unexpected.

1. Consider updating a dependent model in afterSave(); Yii provides isTransactional allowing me to make the afterSave() handler part of the transaction.
2. Consider calling an API / doing some logging in afterSave(); the intuitive assumption is that a record has been saved during afterSave(), but if the transaction is rolled back this is not the case.

Not sure if there's 1 solution that fits both of these scenarios.