Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

afterCommit() callbacks do not fire when RefreshDatabase handles multiple $connectionsToTransact #48472

Closed
mpyw opened this issue Sep 20, 2023 · 6 comments

Comments

@mpyw
Copy link
Contributor

mpyw commented Sep 20, 2023

Laravel Version

10.24.0

PHP Version

irrelevant

Database Driver & Version

irrelevant

Description

When multiple connections are specified in $connectionsToTransact (default value: [null]), any callback functions specified in DB::afterCommit() will not be executed.

Steps To Reproduce

<?php

namespace Tests\Feature;

use Illuminate\Foundation\Testing\RefreshDatabase;
use Illuminate\Support\Facades\DB;
use Tests\TestCase;

class ExampleTest extends TestCase
{
    protected $connectionsToTransact = [null, 'sqlite'];

    use RefreshDatabase;

    public function testExample(): void
    {
        $executed = false;

        DB::transaction(function () use (&$executed) {
            DB::afterCommit(function () use (&$executed) {
                $executed = true;
            });
        });

        $this->assertTrue($executed);
    }
}
@mpyw
Copy link
Contributor Author

mpyw commented Sep 20, 2023

The timing of callback execution looks to be unclear.

  • Without RefreshDatabase, it depends on $transactions === 0 for each connection
  • With RefreshDatabase, the decision may be made over multiple connections

@github-actions
Copy link

Thank you for reporting this issue!

As Laravel is an open source project, we rely on the community to help us diagnose and fix issues as it is not possible to research and fix every issue reported to us via GitHub.

If possible, please make a pull request fixing the issue you have described, along with corresponding tests. All pull requests are promptly reviewed by the Laravel team.

Thank you!

@mpyw
Copy link
Contributor Author

mpyw commented Sep 21, 2023

@tonysm Does #48466 also fix #48472?

@tonysm
Copy link
Contributor

tonysm commented Sep 21, 2023

It should, yeah

@tonysm
Copy link
Contributor

tonysm commented Sep 21, 2023

Wait, not sure. You mentio multiple connections, I haven't tested that.

@mpyw
Copy link
Contributor Author

mpyw commented Sep 27, 2023

Looks to be fixed by #48523

@crynobone Thanks!

@mpyw mpyw closed this as completed Sep 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants