[Proposal] Commit callbacks for Eloquent

[reshadman]

Problem

During a database transaction possible model callbacks with various side effects may be triggered:

<?php

class User extends Model {

    protected static function boot ()
    {
        static::created(function ($user) {

            $user->downloadGravatar();

        });
    }

}

In the above example, The user creation may actually fail due to a transaction rollback outside the scope of the model, but we have downloaded the gravatar for the user, another thing is that expensive processes (like external I/O interaction for downloading gravatar or putting the download command to the queue in the above example) may happen inside an open transaction, which may cause unwanted effects, like hugging connection pool, increasing response times for other requests, etc.

Solution

Adding a committed callback to the Eloquent native event callbacks (saved, created, deleted etc). When ever a new model is saved, updated and deleted, we register model's commit callbacks in to the underlying connection instance, then the database connection fires its callback queues whenever it is committed, or forgets them entirely in case of a rollback. I have implemented a package for this, but I think it is more reliable to handle the commit callbacks inside the ManagesTransactions concern instead of relying on its events, and I think it also deserves to be in the core, as it will lead to less bugs in some cases, and it may improve performance.

<?php

class User extends Model {

    protected static function boot ()
    {
        static::comittedCreation(function ($user) {

            $user->downloadGravatar();

        });

        static::committed(function ($user) {}); // like saved
    }

}

[sisve]

When ever a new model is saved, updated and deleted, we register model's commit callbacks in to the underlying connection instance, [...]

This sentence just describes how you would solve your problem, I presume, while the idea is about the second half;

[...], then the database connection fires its callback queues whenever it is committed, or forgets them entirely in case of a rollback.

1. It would make sense to also add a rollback-event to complement the committed event.
2. This would need to support implicit transactions.
3. Does models have a connection object to register events at?

[reshadman]

@sisve I see, By we I meant the proposal for underlying code not me. I meant whenever an event is fired (saved etc) we will look that is there any commit callbacks registered in the boot, if there is any the model will register them in to the connection to fire them upon commit, something like this:

$this->getConnection()->registerCommitCallback($modelCommitCallback)

The rollback event is fine, committing callbacks may help in some cases, too.

This would need to support implicit transactions.

It wouldn't, Each connection inside a model has a unique name and transaction level this includes connections inside a model, or outside a model for explicit transaction opening,(DB::begingTransaction(), DB::commit()). Also if there is no open transaction for the connection instance of the model (It means transaction level is 0) callbacks will be fired immediately (Take a look at this link).

Does models have a connection object to register events at?

They do have their own connection instance $model->getConnection().

[sisve]

It wouldn't, [...] Also if there is no open transaction for the connection instance of the model (It means transaction level is 0) callbacks will be fired immediately [...]

I think you've misunderstood what an implicit transaction is. You describe it exactly correct. The database will wrap any single statement in a small transaction (either the entire UPDATE ... succeeds, or it fails), this transaction is an implicitly started transaction, while those you start by calling START TRANSACTION are explicit transactions.

At least mysql has an autocommit setting that determines if mysql should automatically commit implicitly started transactions, or wait for you to call COMMIT. This complicates things since it would be up to a server configuration to determine if we should fire our events or not. To be fair, this complicates things for everyone; do you expect a single stand-alone successful DELETE * FROM users; to have emptied the table or not? The autocommit setting will always haunt you...

Do we need to worry about DDL-statements that will generate an implicit commit? These would affect explicit transactions. Things could be committed without the committed event ever being fired.

DB::transaction(function() use ($user) {
    $user->update(array('field' => 'value'));

    // CREATE TABLE issues an implicit commit, which will persist changes to the user model.
    DB::statement('CREATE TABLE shazaam ( id int );');

    // This will fail the callback and issue a rollback. 
    // The rollback would only affect the CREATE TABLE statement.
    // The rollback event will be fired for the $user model, even when it was actually committed.
    throw new Exception('Shazaam!');
});

Can models change the connection they are associated with during their lifecycle? Would we need to move the registered callbacks between connections if that's the case?

[reshadman]

@sisve Thanks for the explanation. I think this will lead to unexpected behaviour even if this feature is entirely disabled. You have done some job inside the closure which you expect to be rolled back upon a failure but they aren't. This can be clarified in the documentation.

Can models change the connection they are associated with during their lifecycle? Would we need to move the registered callbacks between connections if that's the case?

I don't see any benefit of moving callbacks or changing connections during lifecycle, unless you want to solve the above problem (implicit transaction) somehow with this approach?

I also think that the model callbacks should be committed whenever the top parent transaction is actually committed (not in save points), But they should be removed immediately from the callback container whenever a save point is rolled back.

[sisve]

So, we'll document that ...

1. The events fire when Laravel does the commit/rollback (and ignore other sources).
2. The events may not fire correctly in case of implicit commits.
3. The events may not fire correctly in case of DML statements mixed with DDL statements (due to implicit commits)
4. The events may not fire correctly if you change the connection during the model's lifecycle.

Are there anything else that needs to be documented?

[reshadman]

Seems fine. I think this could be added:

5. Events are fire on the final real commit not save points.

[sisve]

I don't think we call them final or real anywhere. How about ...

5. The events will not fire for nested transactions.

[reshadman]

@sisve They would be fired, but whenever the very top one is committed.

[sisve]

I agree. But I don't think the topmost/outer transaction is ever call a "nested transaction", only those that are starting within an existing transaction can become nested transactions. But we could make that point clearer. I'm not happy with the phrase "main transaction", but I don't know if there's a proper term for it.

5. The events will not fire when nested transactions are committed or rolled back, only when the "main" transaction is committed or rolled back.