`DB::afterCommit` callbacks aren't run in tests when `RefreshDatabase` trait is used

[mariomka]

• Laravel Version: v8.21.0
• PHP Version: 7.4.11
• Database Driver & Version: psql (PostgreSQL) 12.4 (Ubuntu 12.4-1.pgdg20.04+1) - It doesn't depend on DB driver.

Description:

DB::afterCommit callbacks aren't run when RefreshDatabase trait is used because ManagesTransactions trait only commits when all transactions are executed, but RefreshDatabase uses a transaction. Then the callbacks aren't called.

Steps To Reproduce:

I've created a reproduction repo: https://github.com/mariomka/laravel-after-commit-repro

There are two tests, one with RefreshDatabase (it fails) and one without RefreshDatabase (it passes): https://github.com/mariomka/laravel-after-commit-repro/tree/master/tests

Both tests run the same "action": https://github.com/mariomka/laravel-after-commit-repro/blob/master/app/AfterCommitAction.php

Step by step reproduction:

git clone git@github.com:mariomka/laravel-after-commit-repro.git
cd laravel-after-commit-repro
cp .env.example .env
composer install
php vendor/bin/homestead make
# Enable postgresql in Homestead.yaml
vagrant up
vagrant ssh
cd code
php vendor/bin/phpunit

[driesvints]

I've installed your repo locally and both tests pass for me. Can you first please try one of the support channels below? If you can actually identify this as a bug, feel free to report back and I'll gladly help you out and re-open this issue.

• Laracasts Forums
• Laravel.io Forums
• StackOverflow
• Discord
• Larachat
• IRC

Thanks!

[mariomka]

@driesvints have you used MySQL or Postgres driver? with SQLite runs because I think it doesn't run transactions

[driesvints]

Ah, you said:

It doesn't depend on DB driver.

So I tried with SQLite indeed.

But still, can you maybe try a support channel first? DB::afterCommit isn't documented atm and I suspect it might just be a misplacement. I'm not 100% sure myself.

[mariomka]

@driesvints I'm Sorry, I didn't think about SQLite.

I'm sure that this is not intended behavior, it works perfectly outside the tests. The issue only occurs in tests.

Checkout this fragment:

framework/src/Illuminate/Database/Concerns/ManagesTransactions.php

Line 43 in 374e956

try {

It only commits when there is only one transaction, if I run the action directly without tests it runs because there is only one transaction but for testing, I need to use RefreshDatabase and it uses a transaction.

There is the line where DatabaseTransactionsManager runs callbacks when a transaction is committed:

framework/src/Illuminate/Database/DatabaseTransactionsManager.php

Line 59 in 374e956

public function commit($connection)

[driesvints]

@mariomka can you maybe try a support channel first? If you can confirm the bug feel free to report back.

[mariomka]

But still, can you maybe try a support channel first? DB::afterCommit isn't documented atm and I suspect it might just be a misplacement. I'm not 100% sure myself.

@driesvints Thank you for your time but I don't know what you want me to ask the community. Seeing some comments on #35373 (#35373 (comment), #35373 (comment), #35373 (comment)) DB::afterCommit should be inside a transaction, then when the transaction is committed callback is called. In fact, it works fine. However, it is untestable when the test needs the RefreshDatabase trait (common when you are using a DB in tests).

[osteel]

Hi,

I've got the same issue. I've just discovered $afterCommit, applied it to one of my observers and now the related tests fail, because yeah, the database transaction is never committed as a result (using a MySQL instance to run my tests too, not SQLite). I guess we'd need a way to ignore $afterCommit in the context of a test suite

[mfn]

I guess we'd need a way to ignore $afterCommit in the context of a test suite

This feels wrong, then you do not know whether something was supposed to be done or not?

[osteel]

This feels wrong, then you do not know whether something was supposed to be done or not?

Not entirely sure what you mean, but since $afterCommit = true needs all transactions to be committed, but Laravel won’t let said transactions get committed in the context of a test suite using MySQL, the only way to test whether what’s supposed to happen once they are committed does happen, would seem to be able to ignore $afterCommit = true in the context of the test suite, somehow.

[mfn]

Wait.

Doesn't it support nested transactions?

Apologies, because I use https://github.com/fntneves/laravel-transactional-events and the transaction opened via the trait (to wrap a test in transactions) does not count as influencing "transactionable ware" code triggered in nested transactions.

(btw I use this together with Postgres with great success).

It would be irritating to me to test a part of an application, which makes use of such a feature and then not knowing whether it was supposed to be executed or not just because the test is wrapped in a transaction. 🤔

[phuclh]

@mariomka I have the same issue when using DB::transaction(), but not when handling the transaction manually with DB::beginTransaction() and DB::commit(). Do you have the answer for this?

[simonmaass]

anyone find a solution for this? i am trying to test a job that creates some models in an DB transaction... and i am trying to test that some jobs are dispatched in the model observer... we are using afterCommit = true in the observer... and they are not being fired

[tylernathanreed]

@simonmaass

I just found this thread, and I didn't see an answer either. I went ahead and solved it myself. For everyone else coming here in the future, here's how I resolved this issue.

The way Laravel handles the "after commit" functionality is by the new DatabaseTransactionsManager introduced in Laravel 8.x. All forms of "after commit" logic funnel into the addCallback method:

public function addCallback($callback)
{
    if ($current = $this->transactions->last()) {
        return $current->addCallback($callback);
    }

    call_user_func($callback);
}

As seen above, if the connection currently has a transaction, all of the callbacks are bound to the ending of that transaction (which in the case of testing, has the scenario where it may never commit).

I overrode the DatabaseTransactionsManager like so:

<?php

namespace App\Providers;

use App\Testing\DatabaseTransactionsManager;
use Illuminate\Support\ServiceProvider;

class TestServiceProvider extends ServiceProvider
{
    /**
     * Register the service provider.
     *
     * @return void
     */
    public function register()
    {
        if (! $this->app->runningUnitTests()) {
            return;
        }

        $this->registerDatabaseTransactionsManager();
    }

    /**
     * Registers the database transaction manager.
     */
    protected function registerDatabaseTransactionsManager()
    {
        // Our automated tests wrap everything inside of a database
        // transactions, which means the "afterCommit" behavior
        // is never invoked. We'll need to tweak this a bit.

        $this->app->singleton('db.transactions', function () {
            return new DatabaseTransactionsManager;
        });
    }
}

This binds my own DatabaseTransactionsManager in place of the original one. Here's what that looks like:

<?php

namespace App\Testing;

use Illuminate\Database\DatabaseTransactionsManager as Manager;

class DatabaseTransactionsManager extends Manager
{
    /**
     * The baseline transaction depth.
     */
    protected $baselineDepth = 0;

    /**
     * Register a transaction callback.
     *
     * @param  callable  $callback
     *
     * @return void
     */
    public function addCallback($callback)
    {
        if ($this->transactions->count() > $this->baselineDepth) {
            return parent::addCallback($callback);
        }

        call_user_func($callback);
    }

    /**
     * Updates the baseline transaction depth to the current transaction depth.
     *
     * @return $this
     */
    public function captureTransactionDepth()
    {
        $this->baselineDepth = $this->transactions->count();

        return $this;
    }
}

The new manager allows you to capture a "baseline depth", of which, upon reaching it, all callbacks are immediately fired, rather than deferred to a transaction committal.

Now you just need to capture the transaction depth in the correct place, which can be done inside of your test:

public function setUp(): void
{
    parent::setUp();

    $this->app->make('db.transactions')->captureTransactionDepth();
}

And with that all in place, any "after commit" calls on top of the refresh transactions will be called immediately. If you have a transaction within your test, the callback will be deferred until the end of that specific transaction, but will still be called at some point in your test.