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

[10.x] Fixed implementation related to afterCommit on Postgres and MSSQL database drivers #48662

Merged
merged 10 commits into from
Oct 9, 2023
11 changes: 5 additions & 6 deletions src/Illuminate/Database/Concerns/ManagesTransactions.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ public function transaction(Closure $callback, $attempts = 1)
$this->getPdo()->commit();
}

$this->transactions = max(0, $this->transactions - 1);

if ($this->afterCommitCallbacksShouldBeExecuted()) {
$this->transactionsManager?->commit($this->getName());
}
Expand All @@ -56,8 +58,6 @@ public function transaction(Closure $callback, $attempts = 1)
);

continue;
} finally {
$this->transactions = max(0, $this->transactions - 1);
}

$this->fireConnectionEvent('committed');
Expand Down Expand Up @@ -194,12 +194,12 @@ public function commit()
$this->getPdo()->commit();
}

$this->transactions = max(0, $this->transactions - 1);

if ($this->afterCommitCallbacksShouldBeExecuted()) {
$this->transactionsManager?->commit($this->getName());
}

$this->transactions = max(0, $this->transactions - 1);

$this->fireConnectionEvent('committed');
}

Expand All @@ -210,8 +210,7 @@ public function commit()
*/
protected function afterCommitCallbacksShouldBeExecuted()
{
return $this->transactions == 0 ||
$this->transactionsManager?->afterCommitCallbacksShouldBeExecuted($this->transactions);
return $this->transactionsManager?->afterCommitCallbacksShouldBeExecuted($this->transactions) || $this->transactions == 0;
}

/**
Expand Down
30 changes: 1 addition & 29 deletions src/Illuminate/Database/DatabaseTransactionsManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,6 @@ class DatabaseTransactionsManager
*/
protected $transactions;

/**
* The database transaction that should be ignored by callbacks.
*
* @var \Illuminate\Database\DatabaseTransactionRecord|null
*/
protected $callbacksShouldIgnore;

/**
* Create a new database transactions manager instance.
*
Expand Down Expand Up @@ -54,10 +47,6 @@ public function rollback($connection, $level)
$this->transactions = $this->transactions->reject(
fn ($transaction) => $transaction->connection == $connection && $transaction->level > $level
)->values();

if ($this->transactions->isEmpty()) {
$this->callbacksShouldIgnore = null;
}
}

/**
Expand All @@ -75,10 +64,6 @@ public function commit($connection)
$this->transactions = $forOtherConnections->values();

$forThisConnection->map->executeCallbacks();

if ($this->transactions->isEmpty()) {
$this->callbacksShouldIgnore = null;
}
}

/**
Expand All @@ -96,19 +81,6 @@ public function addCallback($callback)
$callback();
}

/**
* Specify that callbacks should ignore the given transaction when determining if they should be executed.
*
* @param \Illuminate\Database\DatabaseTransactionRecord $transaction
* @return $this
*/
public function callbacksShouldIgnore(DatabaseTransactionRecord $transaction)
{
$this->callbacksShouldIgnore = $transaction;

return $this;
}

/**
* Get the transactions that are applicable to callbacks.
*
Expand All @@ -127,7 +99,7 @@ public function callbackApplicableTransactions()
*/
public function afterCommitCallbacksShouldBeExecuted($level)
{
return $level === 1;
return $level === 0;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,6 @@ public function callbackApplicableTransactions()
*/
public function afterCommitCallbacksShouldBeExecuted($level)
{
return $level === 2;
return $level === 1;
}
}
15 changes: 15 additions & 0 deletions tests/Database/DatabaseConnectionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use Exception;
use Illuminate\Contracts\Events\Dispatcher;
use Illuminate\Database\Connection;
use Illuminate\Database\DatabaseTransactionsManager;
use Illuminate\Database\Events\QueryExecuted;
use Illuminate\Database\Events\TransactionBeginning;
use Illuminate\Database\Events\TransactionCommitted;
Expand Down Expand Up @@ -289,6 +290,20 @@ public function testCommittingFiresEventsIfSet()
$connection->commit();
}

public function testAfterCommitIsExecutedOnFinalCommit()
{
$pdo = $this->getMockBuilder(DatabaseConnectionTestMockPDO::class)->onlyMethods(['beginTransaction', 'commit'])->getMock();
$transactionsManager = $this->getMockBuilder(DatabaseTransactionsManager::class)->onlyMethods(['afterCommitCallbacksShouldBeExecuted'])->getMock();
$transactionsManager->expects($this->once())->method('afterCommitCallbacksShouldBeExecuted')->with(0)->willReturn(true);

$connection = $this->getMockConnection([], $pdo);
$connection->setTransactionManager($transactionsManager);

$connection->transaction(function () {
// do nothing
});
}

public function testRollBackedFiresEventsIfSet()
{
$pdo = $this->createMock(DatabaseConnectionTestMockPDO::class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@

namespace Illuminate\Tests\Integration\Database;

use Illuminate\Bus\Queueable;
use Illuminate\Contracts\Queue\ShouldQueue;
use Illuminate\Foundation\Auth\User;
use Illuminate\Foundation\Bus\Dispatchable;
use Illuminate\Queue\InteractsWithQueue;
use Illuminate\Support\Facades\DB;
use Orchestra\Testbench\Concerns\WithLaravelMigrations;
use Orchestra\Testbench\Factories\UserFactory;
Expand Down Expand Up @@ -41,6 +45,21 @@ public function testObserverCalledWithAfterCommitWhenInsideTransaction()
$this->assertEquals(1, $observer::$calledTimes, 'Failed to assert the observer was called once.');
}

public function testObserverCalledWithAfterCommitWhenInsideTransactionWithDispatchSync()
{
User::observe($observer = EloquentTransactionWithAfterCommitTestsUserObserverUsingDispatchSync::resetting());

$user1 = DB::transaction(fn () => User::create(UserFactory::new()->raw()));

$this->assertTrue($user1->exists);
$this->assertEquals(1, $observer::$calledTimes, 'Failed to assert the observer was called once.');

$this->assertDatabaseHas('password_reset_tokens', [
'email' => $user1->email,
'token' => sha1($user1->email),
]);
}

public function testObserverIsCalledOnTestsWithAfterCommitWhenUsingSavepoint()
{
User::observe($observer = EloquentTransactionWithAfterCommitTestsUserObserver::resetting());
Expand Down Expand Up @@ -102,3 +121,32 @@ public function created($user)
static::$calledTimes++;
}
}

class EloquentTransactionWithAfterCommitTestsUserObserverUsingDispatchSync extends EloquentTransactionWithAfterCommitTestsUserObserver
{
public function created($user)
{
dispatch_sync(new EloquentTransactionWithAfterCommitTestsJob($user->email));

parent::created($user);
}
}

class EloquentTransactionWithAfterCommitTestsJob implements ShouldQueue
{
use Dispatchable, InteractsWithQueue, Queueable;

public function __construct(public string $email)
{
// ...
}

public function handle(): void
{
DB::transaction(function () {
DB::table('password_reset_tokens')->insert([
['email' => $this->email, 'token' => sha1($this->email), 'created_at' => now()],
]);
});
}
}
Loading