From 1f10b5e639b0537fc780d0c7d0302e8472792308 Mon Sep 17 00:00:00 2001 From: Tony Messias Date: Wed, 20 Sep 2023 00:56:29 -0300 Subject: [PATCH 01/23] Tests observer using afterCommit --- tests/Database/DatabaseEloquentAppTest.php | 76 ++++++++++++++++++++++ 1 file changed, 76 insertions(+) create mode 100644 tests/Database/DatabaseEloquentAppTest.php diff --git a/tests/Database/DatabaseEloquentAppTest.php b/tests/Database/DatabaseEloquentAppTest.php new file mode 100644 index 000000000000..b84f2317ae28 --- /dev/null +++ b/tests/Database/DatabaseEloquentAppTest.php @@ -0,0 +1,76 @@ +id(); + $table->string('email')->unique(); + $table->timestamps(); + }); + } + + public function testObserverIsCalledOnTestsWithAfterCommit() + { + DatabaseEloquentAppTestUser::observe($observer = DatabaseEloquentAppTestUserObserver::reseting()); + + $user1 = DatabaseEloquentAppTestUser::create([ + 'email' => 'hello@example.com', + ]); + + $this->assertTrue($user1->exists); + $this->assertEquals(1, $observer::$calledTimes, 'Failed to assert the observer was called once.'); + } + + public function testObserverCalledWithAfterCommitWhenInsideTransaction() + { + DatabaseEloquentAppTestUser::observe($observer = DatabaseEloquentAppTestUserObserver::reseting()); + + $user1 = DB::transaction(fn () => DatabaseEloquentAppTestUser::create([ + 'email' => 'hello@example.com', + ])); + + $this->assertTrue($user1->exists); + $this->assertEquals(1, $observer::$calledTimes, 'Failed to assert the observer was called once.'); + } +} + +class DatabaseEloquentAppTestUser extends Model +{ + protected $guarded = []; +} + +class DatabaseEloquentAppTestUserObserver +{ + public static $calledTimes = 0; + + public $afterCommit = true; + + public static function reseting() + { + static::$calledTimes = 0; + + return new static(); + } + + public function created($user) + { + static::$calledTimes++; + } +} From d9538d6d007367a1e6c10d8f542c92f6b7731957 Mon Sep 17 00:00:00 2001 From: Tony Messias Date: Wed, 20 Sep 2023 00:59:26 -0300 Subject: [PATCH 02/23] Adds failing test for savepoint and observers using afterCommit --- tests/Database/DatabaseEloquentAppTest.php | 24 ++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/Database/DatabaseEloquentAppTest.php b/tests/Database/DatabaseEloquentAppTest.php index b84f2317ae28..7c18457c078c 100644 --- a/tests/Database/DatabaseEloquentAppTest.php +++ b/tests/Database/DatabaseEloquentAppTest.php @@ -49,6 +49,30 @@ public function testObserverCalledWithAfterCommitWhenInsideTransaction() $this->assertTrue($user1->exists); $this->assertEquals(1, $observer::$calledTimes, 'Failed to assert the observer was called once.'); } + + public function testObserverIsCalledOnTestsWithAfterCommitWhenUsingSavepoint() + { + DatabaseEloquentAppTestUser::observe($observer = DatabaseEloquentAppTestUserObserver::reseting()); + + $user1 = DatabaseEloquentAppTestUser::createOrFirst([ + 'email' => 'hello@example.com', + ]); + + $this->assertTrue($user1->exists); + $this->assertEquals(1, $observer::$calledTimes, 'Failed to assert the observer was called once.'); + } + + public function testObserverIsCalledOnTestsWithAfterCommitWhenUsingSavepointAndInsideTransaction() + { + DatabaseEloquentAppTestUser::observe($observer = DatabaseEloquentAppTestUserObserver::reseting()); + + $user1 = DB::transaction(fn () => DatabaseEloquentAppTestUser::createOrFirst([ + 'email' => 'hello@example.com', + ])); + + $this->assertTrue($user1->exists); + $this->assertEquals(1, $observer::$calledTimes, 'Failed to assert the observer was called once.'); + } } class DatabaseEloquentAppTestUser extends Model From e6de881fd3c4f5479529644a9efae4a5e12ef03f Mon Sep 17 00:00:00 2001 From: Tony Messias Date: Wed, 20 Sep 2023 01:29:06 -0300 Subject: [PATCH 03/23] Ignore the createOrFirst savepoint to run callbacks --- src/Illuminate/Database/Connection.php | 12 ++++++++++++ .../Database/DatabaseTransactionsManager.php | 11 ++++++----- src/Illuminate/Database/Eloquent/Builder.php | 14 +++++++++++--- .../Foundation/Testing/RefreshDatabase.php | 4 +--- 4 files changed, 30 insertions(+), 11 deletions(-) diff --git a/src/Illuminate/Database/Connection.php b/src/Illuminate/Database/Connection.php index eb14815b87b9..8a2e4f1acd10 100755 --- a/src/Illuminate/Database/Connection.php +++ b/src/Illuminate/Database/Connection.php @@ -1545,6 +1545,18 @@ public function unsetTransactionManager() $this->transactionsManager = null; } + /** + * Ignores the current transaction level when calling the callbacks. + * + * @return void + */ + public function ignoreLatestTransactionForCallbacks() + { + $this->transactionsManager?->callbacksShouldIgnore( + $this->transactionsManager?->getTransactions()?->last() + ); + } + /** * Determine if the connection is in a "dry run". * diff --git a/src/Illuminate/Database/DatabaseTransactionsManager.php b/src/Illuminate/Database/DatabaseTransactionsManager.php index 8d145188f065..448ae66e098f 100755 --- a/src/Illuminate/Database/DatabaseTransactionsManager.php +++ b/src/Illuminate/Database/DatabaseTransactionsManager.php @@ -14,7 +14,7 @@ class DatabaseTransactionsManager /** * The database transaction that should be ignored by callbacks. * - * @var \Illuminate\Database\DatabaseTransactionRecord + * @var \Illuminate\Support\Collection */ protected $callbacksShouldIgnore; @@ -26,6 +26,7 @@ class DatabaseTransactionsManager public function __construct() { $this->transactions = collect(); + $this->callbacksShouldIgnore = collect(); } /** @@ -56,7 +57,7 @@ public function rollback($connection, $level) )->values(); if ($this->transactions->isEmpty()) { - $this->callbacksShouldIgnore = null; + $this->callbacksShouldIgnore = collect(); } } @@ -77,7 +78,7 @@ public function commit($connection) $forThisConnection->map->executeCallbacks(); if ($this->transactions->isEmpty()) { - $this->callbacksShouldIgnore = null; + $this->callbacksShouldIgnore = collect(); } } @@ -104,7 +105,7 @@ public function addCallback($callback) */ public function callbacksShouldIgnore(DatabaseTransactionRecord $transaction) { - $this->callbacksShouldIgnore = $transaction; + $this->callbacksShouldIgnore->push($transaction); return $this; } @@ -117,7 +118,7 @@ public function callbacksShouldIgnore(DatabaseTransactionRecord $transaction) public function callbackApplicableTransactions() { return $this->transactions->reject(function ($transaction) { - return $transaction === $this->callbacksShouldIgnore; + return $this->callbacksShouldIgnore->contains($transaction); })->values(); } diff --git a/src/Illuminate/Database/Eloquent/Builder.php b/src/Illuminate/Database/Eloquent/Builder.php index 6d502d6cd118..db016af6e245 100755 --- a/src/Illuminate/Database/Eloquent/Builder.php +++ b/src/Illuminate/Database/Eloquent/Builder.php @@ -1721,9 +1721,17 @@ public function withCasts($casts) */ public function withSavepointIfNeeded(Closure $scope): mixed { - return $this->getQuery()->getConnection()->transactionLevel() > 0 - ? $this->getQuery()->getConnection()->transaction($scope) - : $scope(); + $connection = $this->getQuery()->getConnection(); + + if ($connection->transactionLevel() === 0) { + return $scope(); + } + + return $connection->transaction(function () use ($connection, $scope) { + $connection->ignoreLatestTransactionForCallbacks(); + + return $scope(); + }); } /** diff --git a/src/Illuminate/Foundation/Testing/RefreshDatabase.php b/src/Illuminate/Foundation/Testing/RefreshDatabase.php index b486cbb0ac91..55c0d9f09e1a 100644 --- a/src/Illuminate/Foundation/Testing/RefreshDatabase.php +++ b/src/Illuminate/Foundation/Testing/RefreshDatabase.php @@ -98,9 +98,7 @@ public function beginDatabaseTransaction() $connection->setEventDispatcher($dispatcher); if ($this->app->resolved('db.transactions')) { - $this->app->make('db.transactions')->callbacksShouldIgnore( - $this->app->make('db.transactions')->getTransactions()->first() - ); + $connection->ignoreLatestTransactionForCallbacks(); } } From 06adc98c03bdff2491b85a6b1b7d352669059193 Mon Sep 17 00:00:00 2001 From: Tony Messias Date: Wed, 20 Sep 2023 01:45:53 -0300 Subject: [PATCH 04/23] Fix typo --- tests/Database/DatabaseEloquentAppTest.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/Database/DatabaseEloquentAppTest.php b/tests/Database/DatabaseEloquentAppTest.php index 7c18457c078c..ea97de1f0722 100644 --- a/tests/Database/DatabaseEloquentAppTest.php +++ b/tests/Database/DatabaseEloquentAppTest.php @@ -28,7 +28,7 @@ protected function setUp(): void public function testObserverIsCalledOnTestsWithAfterCommit() { - DatabaseEloquentAppTestUser::observe($observer = DatabaseEloquentAppTestUserObserver::reseting()); + DatabaseEloquentAppTestUser::observe($observer = DatabaseEloquentAppTestUserObserver::resetting()); $user1 = DatabaseEloquentAppTestUser::create([ 'email' => 'hello@example.com', @@ -40,7 +40,7 @@ public function testObserverIsCalledOnTestsWithAfterCommit() public function testObserverCalledWithAfterCommitWhenInsideTransaction() { - DatabaseEloquentAppTestUser::observe($observer = DatabaseEloquentAppTestUserObserver::reseting()); + DatabaseEloquentAppTestUser::observe($observer = DatabaseEloquentAppTestUserObserver::resetting()); $user1 = DB::transaction(fn () => DatabaseEloquentAppTestUser::create([ 'email' => 'hello@example.com', @@ -52,7 +52,7 @@ public function testObserverCalledWithAfterCommitWhenInsideTransaction() public function testObserverIsCalledOnTestsWithAfterCommitWhenUsingSavepoint() { - DatabaseEloquentAppTestUser::observe($observer = DatabaseEloquentAppTestUserObserver::reseting()); + DatabaseEloquentAppTestUser::observe($observer = DatabaseEloquentAppTestUserObserver::resetting()); $user1 = DatabaseEloquentAppTestUser::createOrFirst([ 'email' => 'hello@example.com', @@ -64,7 +64,7 @@ public function testObserverIsCalledOnTestsWithAfterCommitWhenUsingSavepoint() public function testObserverIsCalledOnTestsWithAfterCommitWhenUsingSavepointAndInsideTransaction() { - DatabaseEloquentAppTestUser::observe($observer = DatabaseEloquentAppTestUserObserver::reseting()); + DatabaseEloquentAppTestUser::observe($observer = DatabaseEloquentAppTestUserObserver::resetting()); $user1 = DB::transaction(fn () => DatabaseEloquentAppTestUser::createOrFirst([ 'email' => 'hello@example.com', @@ -86,7 +86,7 @@ class DatabaseEloquentAppTestUserObserver public $afterCommit = true; - public static function reseting() + public static function resetting() { static::$calledTimes = 0; From bf2764bb4acf04d8c1e34a9055d8d78a17f897a3 Mon Sep 17 00:00:00 2001 From: Tony Messias Date: Wed, 20 Sep 2023 01:59:47 -0300 Subject: [PATCH 05/23] Remove DatabaseEloquentAppTest to see if CI passes --- tests/Database/DatabaseEloquentAppTest.php | 100 --------------------- 1 file changed, 100 deletions(-) delete mode 100644 tests/Database/DatabaseEloquentAppTest.php diff --git a/tests/Database/DatabaseEloquentAppTest.php b/tests/Database/DatabaseEloquentAppTest.php deleted file mode 100644 index ea97de1f0722..000000000000 --- a/tests/Database/DatabaseEloquentAppTest.php +++ /dev/null @@ -1,100 +0,0 @@ -id(); - $table->string('email')->unique(); - $table->timestamps(); - }); - } - - public function testObserverIsCalledOnTestsWithAfterCommit() - { - DatabaseEloquentAppTestUser::observe($observer = DatabaseEloquentAppTestUserObserver::resetting()); - - $user1 = DatabaseEloquentAppTestUser::create([ - 'email' => 'hello@example.com', - ]); - - $this->assertTrue($user1->exists); - $this->assertEquals(1, $observer::$calledTimes, 'Failed to assert the observer was called once.'); - } - - public function testObserverCalledWithAfterCommitWhenInsideTransaction() - { - DatabaseEloquentAppTestUser::observe($observer = DatabaseEloquentAppTestUserObserver::resetting()); - - $user1 = DB::transaction(fn () => DatabaseEloquentAppTestUser::create([ - 'email' => 'hello@example.com', - ])); - - $this->assertTrue($user1->exists); - $this->assertEquals(1, $observer::$calledTimes, 'Failed to assert the observer was called once.'); - } - - public function testObserverIsCalledOnTestsWithAfterCommitWhenUsingSavepoint() - { - DatabaseEloquentAppTestUser::observe($observer = DatabaseEloquentAppTestUserObserver::resetting()); - - $user1 = DatabaseEloquentAppTestUser::createOrFirst([ - 'email' => 'hello@example.com', - ]); - - $this->assertTrue($user1->exists); - $this->assertEquals(1, $observer::$calledTimes, 'Failed to assert the observer was called once.'); - } - - public function testObserverIsCalledOnTestsWithAfterCommitWhenUsingSavepointAndInsideTransaction() - { - DatabaseEloquentAppTestUser::observe($observer = DatabaseEloquentAppTestUserObserver::resetting()); - - $user1 = DB::transaction(fn () => DatabaseEloquentAppTestUser::createOrFirst([ - 'email' => 'hello@example.com', - ])); - - $this->assertTrue($user1->exists); - $this->assertEquals(1, $observer::$calledTimes, 'Failed to assert the observer was called once.'); - } -} - -class DatabaseEloquentAppTestUser extends Model -{ - protected $guarded = []; -} - -class DatabaseEloquentAppTestUserObserver -{ - public static $calledTimes = 0; - - public $afterCommit = true; - - public static function resetting() - { - static::$calledTimes = 0; - - return new static(); - } - - public function created($user) - { - static::$calledTimes++; - } -} From 451fd8bcc9667c059c882e972f1587f0b89d913f Mon Sep 17 00:00:00 2001 From: Mior Muhammad Zaki Date: Wed, 20 Sep 2023 14:21:40 +0800 Subject: [PATCH 06/23] wip Signed-off-by: Mior Muhammad Zaki --- ...entTransactionUsingRefreshDatabaseTest.php | 89 +++++++++++++++++++ 1 file changed, 89 insertions(+) create mode 100644 tests/Integration/Database/EloquentTransactionUsingRefreshDatabaseTest.php diff --git a/tests/Integration/Database/EloquentTransactionUsingRefreshDatabaseTest.php b/tests/Integration/Database/EloquentTransactionUsingRefreshDatabaseTest.php new file mode 100644 index 000000000000..a711916bb3c2 --- /dev/null +++ b/tests/Integration/Database/EloquentTransactionUsingRefreshDatabaseTest.php @@ -0,0 +1,89 @@ +afterApplicationCreated(fn () => User::unguard()); + $this->beforeApplicationDestroyed(fn () => User::reguard()); + + parent::setUp(); + } + + public function testObserverIsCalledOnTestsWithAfterCommit() + { + User::observe($observer = EloquentTransactionUsingRefreshDatabaseUserObserver::resetting()); + + $user1 = User::create($this->newFakeUser()); + + $this->assertTrue($user1->exists); + $this->assertEquals(1, $observer::$calledTimes, 'Failed to assert the observer was called once.'); + } + + public function testObserverCalledWithAfterCommitWhenInsideTransaction() + { + User::observe($observer = EloquentTransactionUsingRefreshDatabaseUserObserver::resetting()); + + $user1 = DB::transaction(fn () => User::create($this->newFakeUser())); + + $this->assertTrue($user1->exists); + $this->assertEquals(1, $observer::$calledTimes, 'Failed to assert the observer was called once.'); + } + + public function testObserverIsCalledOnTestsWithAfterCommitWhenUsingSavepoint() + { + User::observe($observer = EloquentTransactionUsingRefreshDatabaseUserObserver::resetting()); + + $user1 = User::createOrFirst($this->newFakeUser()); + + $this->assertTrue($user1->exists); + $this->assertEquals(1, $observer::$calledTimes, 'Failed to assert the observer was called once.'); + } + + public function testObserverIsCalledOnTestsWithAfterCommitWhenUsingSavepointAndInsideTransaction() + { + User::observe($observer = EloquentTransactionUsingRefreshDatabaseUserObserver::resetting()); + + $user1 = DB::transaction(fn () => User::createOrFirst($this->newFakeUser())); + + $this->assertTrue($user1->exists); + $this->assertEquals(1, $observer::$calledTimes, 'Failed to assert the observer was called once.'); + } + + protected function newFakeUser(): array + { + return UserFactory::new()->make()->makeVisible('password')->toArray(); + } +} + +class EloquentTransactionUsingRefreshDatabaseUserObserver +{ + public static $calledTimes = 0; + + public $afterCommit = true; + + public static function resetting() + { + static::$calledTimes = 0; + + return new static(); + } + + public function created($user) + { + static::$calledTimes++; + } +} From f49260e8d1d8ab6360bba58e21a2420a2d6afedf Mon Sep 17 00:00:00 2001 From: Mior Muhammad Zaki Date: Wed, 20 Sep 2023 14:22:42 +0800 Subject: [PATCH 07/23] wip Signed-off-by: Mior Muhammad Zaki --- .../Database/EloquentTransactionUsingRefreshDatabaseTest.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/Integration/Database/EloquentTransactionUsingRefreshDatabaseTest.php b/tests/Integration/Database/EloquentTransactionUsingRefreshDatabaseTest.php index a711916bb3c2..51d81278228c 100644 --- a/tests/Integration/Database/EloquentTransactionUsingRefreshDatabaseTest.php +++ b/tests/Integration/Database/EloquentTransactionUsingRefreshDatabaseTest.php @@ -2,12 +2,9 @@ namespace Illuminate\Tests\Integration\Database; -use Illuminate\Database\Eloquent\Model; -use Illuminate\Database\Schema\Blueprint; use Illuminate\Foundation\Auth\User; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Facades\DB; -use Illuminate\Support\Facades\Schema; use Orchestra\Testbench\Concerns\WithLaravelMigrations; use Orchestra\Testbench\Factories\UserFactory; From a7a2fa2949bc066aab088df27e0e06da83a2e4f0 Mon Sep 17 00:00:00 2001 From: Mior Muhammad Zaki Date: Wed, 20 Sep 2023 14:28:09 +0800 Subject: [PATCH 08/23] wip Signed-off-by: Mior Muhammad Zaki --- .../EloquentTransactionUsingRefreshDatabaseTest.php | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/tests/Integration/Database/EloquentTransactionUsingRefreshDatabaseTest.php b/tests/Integration/Database/EloquentTransactionUsingRefreshDatabaseTest.php index 51d81278228c..8e030643db0c 100644 --- a/tests/Integration/Database/EloquentTransactionUsingRefreshDatabaseTest.php +++ b/tests/Integration/Database/EloquentTransactionUsingRefreshDatabaseTest.php @@ -24,7 +24,7 @@ public function testObserverIsCalledOnTestsWithAfterCommit() { User::observe($observer = EloquentTransactionUsingRefreshDatabaseUserObserver::resetting()); - $user1 = User::create($this->newFakeUser()); + $user1 = User::create(UserFactory::new()->raw()); $this->assertTrue($user1->exists); $this->assertEquals(1, $observer::$calledTimes, 'Failed to assert the observer was called once.'); @@ -34,7 +34,7 @@ public function testObserverCalledWithAfterCommitWhenInsideTransaction() { User::observe($observer = EloquentTransactionUsingRefreshDatabaseUserObserver::resetting()); - $user1 = DB::transaction(fn () => User::create($this->newFakeUser())); + $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.'); @@ -44,7 +44,7 @@ public function testObserverIsCalledOnTestsWithAfterCommitWhenUsingSavepoint() { User::observe($observer = EloquentTransactionUsingRefreshDatabaseUserObserver::resetting()); - $user1 = User::createOrFirst($this->newFakeUser()); + $user1 = User::createOrFirst(UserFactory::new()->raw()); $this->assertTrue($user1->exists); $this->assertEquals(1, $observer::$calledTimes, 'Failed to assert the observer was called once.'); @@ -54,16 +54,11 @@ public function testObserverIsCalledOnTestsWithAfterCommitWhenUsingSavepointAndI { User::observe($observer = EloquentTransactionUsingRefreshDatabaseUserObserver::resetting()); - $user1 = DB::transaction(fn () => User::createOrFirst($this->newFakeUser())); + $user1 = DB::transaction(fn () => User::createOrFirst(UserFactory::new()->raw())); $this->assertTrue($user1->exists); $this->assertEquals(1, $observer::$calledTimes, 'Failed to assert the observer was called once.'); } - - protected function newFakeUser(): array - { - return UserFactory::new()->make()->makeVisible('password')->toArray(); - } } class EloquentTransactionUsingRefreshDatabaseUserObserver From e3df8b1f27983bca15ffc7b396e72e9e96cdfa86 Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Wed, 20 Sep 2023 10:01:30 -0500 Subject: [PATCH 09/23] Update src/Illuminate/Database/DatabaseTransactionsManager.php Co-authored-by: Mior Muhammad Zaki --- src/Illuminate/Database/DatabaseTransactionsManager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Illuminate/Database/DatabaseTransactionsManager.php b/src/Illuminate/Database/DatabaseTransactionsManager.php index 448ae66e098f..4b9ae41f971d 100755 --- a/src/Illuminate/Database/DatabaseTransactionsManager.php +++ b/src/Illuminate/Database/DatabaseTransactionsManager.php @@ -14,7 +14,7 @@ class DatabaseTransactionsManager /** * The database transaction that should be ignored by callbacks. * - * @var \Illuminate\Support\Collection + * @var \Illuminate\Support\Collection */ protected $callbacksShouldIgnore; From f5aacf5b9a7a593a95d7c4d979fb66712ba60b4f Mon Sep 17 00:00:00 2001 From: Tony Messias Date: Wed, 20 Sep 2023 21:37:38 -0300 Subject: [PATCH 10/23] Changes the way the after commit callbacks are executed to avoid remembering which transactions to ignore Before, we were remembering the test transaction so we could ignore it when deciding to run the after commit callbacks or not. We're still handling the after commit callbacks like that but now instead of remembering which transactions to ignore, we're always calling the DatabaseTransactionManager::commit method. The difference is that now we're passing the current transaction level to it. The method will decide to call the callbacks or not based on that level and whether or not this is on in test mode. When in tests, instead of setting the current transaction to be remembered so it could be ignored, we're now only setting the DatabaseTransactionManager to test mode. When in test mode, it will execute the callbacks when the transactions count reaches 1 (remember that the test runs in a transaction, so that's the "root" level). Otherwise, it runs the callbacks when the transactions level is on level 0 (like in production). There's also a change in the DatabaseTransactionManager::addCallback method. It now also checks if it's in test mode. When not in test mode, it only adds the callback to the execution queue if there's an open transaction. Otherwise, the callback is executed right away. When in test mode, the number of transactions has to be greater than one for it to be added to the callbacks queue. --- .../Database/Concerns/ManagesTransactions.php | 20 +---- src/Illuminate/Database/Connection.php | 12 --- .../Database/DatabaseTransactionsManager.php | 80 +++++++++---------- src/Illuminate/Database/Eloquent/Builder.php | 14 +--- .../Testing/DatabaseTransactions.php | 4 +- .../Foundation/Testing/RefreshDatabase.php | 2 +- 6 files changed, 43 insertions(+), 89 deletions(-) diff --git a/src/Illuminate/Database/Concerns/ManagesTransactions.php b/src/Illuminate/Database/Concerns/ManagesTransactions.php index 14661cc76ebf..7b6bbbc9b1ea 100644 --- a/src/Illuminate/Database/Concerns/ManagesTransactions.php +++ b/src/Illuminate/Database/Concerns/ManagesTransactions.php @@ -49,9 +49,7 @@ public function transaction(Closure $callback, $attempts = 1) $this->transactions = max(0, $this->transactions - 1); - if ($this->afterCommitCallbacksShouldBeExecuted()) { - $this->transactionsManager?->commit($this->getName()); - } + $this->transactionsManager?->commit($this->getName(), $this->transactions); } catch (Throwable $e) { $this->handleCommitTransactionException( $e, $currentAttempt, $attempts @@ -196,25 +194,11 @@ public function commit() $this->transactions = max(0, $this->transactions - 1); - if ($this->afterCommitCallbacksShouldBeExecuted()) { - $this->transactionsManager?->commit($this->getName()); - } + $this->transactionsManager?->commit($this->getName(), $this->transactions); $this->fireConnectionEvent('committed'); } - /** - * Determine if after commit callbacks should be executed. - * - * @return bool - */ - protected function afterCommitCallbacksShouldBeExecuted() - { - return $this->transactions == 0 || - ($this->transactionsManager && - $this->transactionsManager->callbackApplicableTransactions()->count() === 1); - } - /** * Handle an exception encountered when committing a transaction. * diff --git a/src/Illuminate/Database/Connection.php b/src/Illuminate/Database/Connection.php index 8a2e4f1acd10..eb14815b87b9 100755 --- a/src/Illuminate/Database/Connection.php +++ b/src/Illuminate/Database/Connection.php @@ -1545,18 +1545,6 @@ public function unsetTransactionManager() $this->transactionsManager = null; } - /** - * Ignores the current transaction level when calling the callbacks. - * - * @return void - */ - public function ignoreLatestTransactionForCallbacks() - { - $this->transactionsManager?->callbacksShouldIgnore( - $this->transactionsManager?->getTransactions()?->last() - ); - } - /** * Determine if the connection is in a "dry run". * diff --git a/src/Illuminate/Database/DatabaseTransactionsManager.php b/src/Illuminate/Database/DatabaseTransactionsManager.php index 4b9ae41f971d..a0824df195d9 100755 --- a/src/Illuminate/Database/DatabaseTransactionsManager.php +++ b/src/Illuminate/Database/DatabaseTransactionsManager.php @@ -12,11 +12,11 @@ class DatabaseTransactionsManager protected $transactions; /** - * The database transaction that should be ignored by callbacks. + * When in test mode, we'll run the after commit callbacks on the top-level transaction. * - * @var \Illuminate\Support\Collection + * @var bool */ - protected $callbacksShouldIgnore; + protected $callbacksTransactionManagerTestMode = false; /** * Create a new database transactions manager instance. @@ -26,7 +26,19 @@ class DatabaseTransactionsManager public function __construct() { $this->transactions = collect(); - $this->callbacksShouldIgnore = collect(); + $this->callbacksTransactionManagerTestMode = false; + } + + /** + * Sets the transaction manager to test mode. + * + * @return self + */ + public function withCallbacksExecutionInTestMode() + { + $this->callbacksTransactionManagerTestMode = true; + + return $this; } /** @@ -55,71 +67,51 @@ 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 = collect(); - } } /** * Commit the active database transaction. * * @param string $connection + * @param int $level * @return void */ - public function commit($connection) + public function commit($connection, $level) { - [$forThisConnection, $forOtherConnections] = $this->transactions->partition( - fn ($transaction) => $transaction->connection == $connection - ); - - $this->transactions = $forOtherConnections->values(); + if ($level == 0 || ($this->isRunningInTestMode() && $level == 1)) { + [$forThisConnection, $forOtherConnections] = $this->transactions->partition( + fn ($transaction) => $transaction->connection == $connection + ); - $forThisConnection->map->executeCallbacks(); + $this->transactions = $forOtherConnections->values(); - if ($this->transactions->isEmpty()) { - $this->callbacksShouldIgnore = collect(); + $forThisConnection->map->executeCallbacks(); } } /** - * Register a transaction callback. + * Checks if the transaction manager is running in test mode. * - * @param callable $callback - * @return void + * @return bool */ - public function addCallback($callback) + public function isRunningInTestMode() { - if ($current = $this->callbackApplicableTransactions()->last()) { - return $current->addCallback($callback); - } - - $callback(); + return $this->callbacksTransactionManagerTestMode; } /** - * Specify that callbacks should ignore the given transaction when determining if they should be executed. + * Register a transaction callback. * - * @param \Illuminate\Database\DatabaseTransactionRecord $transaction - * @return $this + * @param callable $callback + * @return void */ - public function callbacksShouldIgnore(DatabaseTransactionRecord $transaction) + public function addCallback($callback) { - $this->callbacksShouldIgnore->push($transaction); - - return $this; - } + if ($this->transactions->isEmpty() || ($this->isRunningInTestMode() && $this->transactions->count() == 1)) { + return $callback(); + } - /** - * Get the transactions that are applicable to callbacks. - * - * @return \Illuminate\Support\Collection - */ - public function callbackApplicableTransactions() - { - return $this->transactions->reject(function ($transaction) { - return $this->callbacksShouldIgnore->contains($transaction); - })->values(); + return $this->transactions->last()->addCallback($callback); } /** diff --git a/src/Illuminate/Database/Eloquent/Builder.php b/src/Illuminate/Database/Eloquent/Builder.php index db016af6e245..6d502d6cd118 100755 --- a/src/Illuminate/Database/Eloquent/Builder.php +++ b/src/Illuminate/Database/Eloquent/Builder.php @@ -1721,17 +1721,9 @@ public function withCasts($casts) */ public function withSavepointIfNeeded(Closure $scope): mixed { - $connection = $this->getQuery()->getConnection(); - - if ($connection->transactionLevel() === 0) { - return $scope(); - } - - return $connection->transaction(function () use ($connection, $scope) { - $connection->ignoreLatestTransactionForCallbacks(); - - return $scope(); - }); + return $this->getQuery()->getConnection()->transactionLevel() > 0 + ? $this->getQuery()->getConnection()->transaction($scope) + : $scope(); } /** diff --git a/src/Illuminate/Foundation/Testing/DatabaseTransactions.php b/src/Illuminate/Foundation/Testing/DatabaseTransactions.php index d0e0bafc52de..66cf4a54bef2 100644 --- a/src/Illuminate/Foundation/Testing/DatabaseTransactions.php +++ b/src/Illuminate/Foundation/Testing/DatabaseTransactions.php @@ -22,9 +22,7 @@ public function beginDatabaseTransaction() $connection->setEventDispatcher($dispatcher); if ($this->app->resolved('db.transactions')) { - $this->app->make('db.transactions')->callbacksShouldIgnore( - $this->app->make('db.transactions')->getTransactions()->first() - ); + $this->app->make('db.transactions')->withCallbacksExecutionInTestMode(); } } diff --git a/src/Illuminate/Foundation/Testing/RefreshDatabase.php b/src/Illuminate/Foundation/Testing/RefreshDatabase.php index 55c0d9f09e1a..4880664fa35b 100644 --- a/src/Illuminate/Foundation/Testing/RefreshDatabase.php +++ b/src/Illuminate/Foundation/Testing/RefreshDatabase.php @@ -98,7 +98,7 @@ public function beginDatabaseTransaction() $connection->setEventDispatcher($dispatcher); if ($this->app->resolved('db.transactions')) { - $connection->ignoreLatestTransactionForCallbacks(); + $this->app->make('db.transactions')->withCallbacksExecutionInTestMode(); } } From 1d7b8ee0b1cac9b72d542adb478b7d9274b46166 Mon Sep 17 00:00:00 2001 From: Tony Messias Date: Wed, 20 Sep 2023 22:00:34 -0300 Subject: [PATCH 11/23] Fix DatabaseTransactionsTest --- .../Database/Concerns/ManagesTransactions.php | 8 ++++---- .../Database/DatabaseTransactionsManager.php | 5 ++++- tests/Database/DatabaseTransactionsTest.php | 12 +++++++----- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/Illuminate/Database/Concerns/ManagesTransactions.php b/src/Illuminate/Database/Concerns/ManagesTransactions.php index 7b6bbbc9b1ea..04209df61afd 100644 --- a/src/Illuminate/Database/Concerns/ManagesTransactions.php +++ b/src/Illuminate/Database/Concerns/ManagesTransactions.php @@ -47,9 +47,9 @@ public function transaction(Closure $callback, $attempts = 1) $this->getPdo()->commit(); } - $this->transactions = max(0, $this->transactions - 1); - $this->transactionsManager?->commit($this->getName(), $this->transactions); + + $this->transactions = max(0, $this->transactions - 1); } catch (Throwable $e) { $this->handleCommitTransactionException( $e, $currentAttempt, $attempts @@ -192,10 +192,10 @@ public function commit() $this->getPdo()->commit(); } - $this->transactions = max(0, $this->transactions - 1); - $this->transactionsManager?->commit($this->getName(), $this->transactions); + $this->transactions = max(0, $this->transactions - 1); + $this->fireConnectionEvent('committed'); } diff --git a/src/Illuminate/Database/DatabaseTransactionsManager.php b/src/Illuminate/Database/DatabaseTransactionsManager.php index a0824df195d9..516bf04defba 100755 --- a/src/Illuminate/Database/DatabaseTransactionsManager.php +++ b/src/Illuminate/Database/DatabaseTransactionsManager.php @@ -78,7 +78,10 @@ public function rollback($connection, $level) */ public function commit($connection, $level) { - if ($level == 0 || ($this->isRunningInTestMode() && $level == 1)) { + // If the transaction level being commited reaches 1 (meaning it was the root + // transaction), we'll run the callbacks. In test mode, since we wrap each + // test in a transaction, we'll run the callbacks when reaching level 2. + if ($level == 1 || ($this->isRunningInTestMode() && $level == 2)) { [$forThisConnection, $forOtherConnections] = $this->transactions->partition( fn ($transaction) => $transaction->connection == $connection ); diff --git a/tests/Database/DatabaseTransactionsTest.php b/tests/Database/DatabaseTransactionsTest.php index ac9587419eeb..8c5166506147 100644 --- a/tests/Database/DatabaseTransactionsTest.php +++ b/tests/Database/DatabaseTransactionsTest.php @@ -64,7 +64,7 @@ public function testTransactionIsRecordedAndCommitted() { $transactionManager = m::mock(new DatabaseTransactionsManager); $transactionManager->shouldReceive('begin')->once()->with('default', 1); - $transactionManager->shouldReceive('commit')->once()->with('default'); + $transactionManager->shouldReceive('commit')->once()->with('default', 1); $this->connection()->setTransactionManager($transactionManager); @@ -83,7 +83,7 @@ public function testTransactionIsRecordedAndCommittedUsingTheSeparateMethods() { $transactionManager = m::mock(new DatabaseTransactionsManager); $transactionManager->shouldReceive('begin')->once()->with('default', 1); - $transactionManager->shouldReceive('commit')->once()->with('default'); + $transactionManager->shouldReceive('commit')->once()->with('default', 1); $this->connection()->setTransactionManager($transactionManager); @@ -103,7 +103,8 @@ public function testNestedTransactionIsRecordedAndCommitted() $transactionManager = m::mock(new DatabaseTransactionsManager); $transactionManager->shouldReceive('begin')->once()->with('default', 1); $transactionManager->shouldReceive('begin')->once()->with('default', 2); - $transactionManager->shouldReceive('commit')->once()->with('default'); + $transactionManager->shouldReceive('commit')->once()->with('default', 2); + $transactionManager->shouldReceive('commit')->once()->with('default', 1); $this->connection()->setTransactionManager($transactionManager); @@ -130,8 +131,9 @@ public function testNestedTransactionIsRecordeForDifferentConnectionsdAndCommitt $transactionManager->shouldReceive('begin')->once()->with('default', 1); $transactionManager->shouldReceive('begin')->once()->with('second_connection', 1); $transactionManager->shouldReceive('begin')->once()->with('second_connection', 2); - $transactionManager->shouldReceive('commit')->once()->with('default'); - $transactionManager->shouldReceive('commit')->once()->with('second_connection'); + $transactionManager->shouldReceive('commit')->once()->with('second_connection', 2); + $transactionManager->shouldReceive('commit')->once()->with('second_connection', 1); + $transactionManager->shouldReceive('commit')->once()->with('default', 1); $this->connection()->setTransactionManager($transactionManager); $this->connection('second_connection')->setTransactionManager($transactionManager); From 65c75ae240eb39e1f063d617320d266c2024e079 Mon Sep 17 00:00:00 2001 From: Tony Messias Date: Wed, 20 Sep 2023 22:09:18 -0300 Subject: [PATCH 12/23] Fix DatabaseTransactionsManagerTest --- tests/Database/DatabaseTransactionsManagerTest.php | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/tests/Database/DatabaseTransactionsManagerTest.php b/tests/Database/DatabaseTransactionsManagerTest.php index e8d82e048720..dbfa21f5b1f1 100755 --- a/tests/Database/DatabaseTransactionsManagerTest.php +++ b/tests/Database/DatabaseTransactionsManagerTest.php @@ -67,7 +67,7 @@ public function testCommittingTransactions() $manager->begin('default', 2); $manager->begin('admin', 1); - $manager->commit('default'); + $manager->commit('default', 1); $this->assertCount(1, $manager->getTransactions()); @@ -118,7 +118,11 @@ public function testCommittingTransactionsExecutesCallbacks() $manager->begin('admin', 1); - $manager->commit('default'); + $manager->commit('default', 2); + + $this->assertEmpty($callbacks, 'Should not have ran the callbacks.'); + + $manager->commit('default', 1); $this->assertCount(2, $callbacks); $this->assertEquals(['default', 1], $callbacks[0]); @@ -144,7 +148,11 @@ public function testCommittingExecutesOnlyCallbacksOfTheConnection() $callbacks[] = ['admin', 1]; }); - $manager->commit('default'); + $manager->commit('default', 2); + + $this->assertEmpty($callbacks, 'Should not have run the callbacks.'); + + $manager->commit('default', 1); $this->assertCount(1, $callbacks); $this->assertEquals(['default', 1], $callbacks[0]); From 2037453a95fa268bcc55d7f3905b2bf41324b1f2 Mon Sep 17 00:00:00 2001 From: Tony Messias Date: Wed, 20 Sep 2023 22:13:30 -0300 Subject: [PATCH 13/23] CSFixer --- src/Illuminate/Database/DatabaseTransactionsManager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Illuminate/Database/DatabaseTransactionsManager.php b/src/Illuminate/Database/DatabaseTransactionsManager.php index 516bf04defba..82b273459848 100755 --- a/src/Illuminate/Database/DatabaseTransactionsManager.php +++ b/src/Illuminate/Database/DatabaseTransactionsManager.php @@ -73,7 +73,7 @@ public function rollback($connection, $level) * Commit the active database transaction. * * @param string $connection - * @param int $level + * @param int $level * @return void */ public function commit($connection, $level) From c7c7f3a01b130327908e54046bb6928d14c2f7db Mon Sep 17 00:00:00 2001 From: Tony Messias Date: Wed, 20 Sep 2023 22:32:02 -0300 Subject: [PATCH 14/23] wip --- .../Database/DatabaseTransactionsManager.php | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/Illuminate/Database/DatabaseTransactionsManager.php b/src/Illuminate/Database/DatabaseTransactionsManager.php index 82b273459848..096fbc2ea8c1 100755 --- a/src/Illuminate/Database/DatabaseTransactionsManager.php +++ b/src/Illuminate/Database/DatabaseTransactionsManager.php @@ -92,16 +92,6 @@ public function commit($connection, $level) } } - /** - * Checks if the transaction manager is running in test mode. - * - * @return bool - */ - public function isRunningInTestMode() - { - return $this->callbacksTransactionManagerTestMode; - } - /** * Register a transaction callback. * @@ -110,6 +100,9 @@ public function isRunningInTestMode() */ public function addCallback($callback) { + // If there are no transactions, we'll run the callbacks right away. Also, we'll run it + // right away when we're in test mode and we only have the wrapping transaction. For + // every other case, we'll queue up the callback to run after the commit happens. if ($this->transactions->isEmpty() || ($this->isRunningInTestMode() && $this->transactions->count() == 1)) { return $callback(); } @@ -126,4 +119,14 @@ public function getTransactions() { return $this->transactions; } + + /** + * Checks if the transaction manager is running in test mode. + * + * @return bool + */ + protected function isRunningInTestMode() + { + return $this->callbacksTransactionManagerTestMode; + } } From 0d8f1faec08b4dccf84aa0d5733305559ea47220 Mon Sep 17 00:00:00 2001 From: Tony Messias Date: Wed, 20 Sep 2023 22:36:58 -0300 Subject: [PATCH 15/23] Rename method and property and inline usage --- .../Database/DatabaseTransactionsManager.php | 22 +++++-------------- .../Foundation/Testing/RefreshDatabase.php | 2 +- 2 files changed, 7 insertions(+), 17 deletions(-) diff --git a/src/Illuminate/Database/DatabaseTransactionsManager.php b/src/Illuminate/Database/DatabaseTransactionsManager.php index 096fbc2ea8c1..cedea759557f 100755 --- a/src/Illuminate/Database/DatabaseTransactionsManager.php +++ b/src/Illuminate/Database/DatabaseTransactionsManager.php @@ -16,7 +16,7 @@ class DatabaseTransactionsManager * * @var bool */ - protected $callbacksTransactionManagerTestMode = false; + protected $afterCommitCallbacksInTestMode = false; /** * Create a new database transactions manager instance. @@ -26,7 +26,7 @@ class DatabaseTransactionsManager public function __construct() { $this->transactions = collect(); - $this->callbacksTransactionManagerTestMode = false; + $this->afterCommitCallbacksInTestMode = false; } /** @@ -34,9 +34,9 @@ public function __construct() * * @return self */ - public function withCallbacksExecutionInTestMode() + public function withAfterCommitCallbacksInTestMode() { - $this->callbacksTransactionManagerTestMode = true; + $this->afterCommitCallbacksInTestMode = true; return $this; } @@ -81,7 +81,7 @@ public function commit($connection, $level) // If the transaction level being commited reaches 1 (meaning it was the root // transaction), we'll run the callbacks. In test mode, since we wrap each // test in a transaction, we'll run the callbacks when reaching level 2. - if ($level == 1 || ($this->isRunningInTestMode() && $level == 2)) { + if ($level == 1 || ($this->afterCommitCallbacksInTestMode && $level == 2)) { [$forThisConnection, $forOtherConnections] = $this->transactions->partition( fn ($transaction) => $transaction->connection == $connection ); @@ -103,7 +103,7 @@ public function addCallback($callback) // If there are no transactions, we'll run the callbacks right away. Also, we'll run it // right away when we're in test mode and we only have the wrapping transaction. For // every other case, we'll queue up the callback to run after the commit happens. - if ($this->transactions->isEmpty() || ($this->isRunningInTestMode() && $this->transactions->count() == 1)) { + if ($this->transactions->isEmpty() || ($this->afterCommitCallbacksInTestMode && $this->transactions->count() == 1)) { return $callback(); } @@ -119,14 +119,4 @@ public function getTransactions() { return $this->transactions; } - - /** - * Checks if the transaction manager is running in test mode. - * - * @return bool - */ - protected function isRunningInTestMode() - { - return $this->callbacksTransactionManagerTestMode; - } } diff --git a/src/Illuminate/Foundation/Testing/RefreshDatabase.php b/src/Illuminate/Foundation/Testing/RefreshDatabase.php index 4880664fa35b..e04b3769697e 100644 --- a/src/Illuminate/Foundation/Testing/RefreshDatabase.php +++ b/src/Illuminate/Foundation/Testing/RefreshDatabase.php @@ -98,7 +98,7 @@ public function beginDatabaseTransaction() $connection->setEventDispatcher($dispatcher); if ($this->app->resolved('db.transactions')) { - $this->app->make('db.transactions')->withCallbacksExecutionInTestMode(); + $this->app->make('db.transactions')->withAfterCommitCallbacksInTestMode(); } } From 393a69c13ea162bbfd8e6893681fcfd7a3ae4422 Mon Sep 17 00:00:00 2001 From: Tony Messias Date: Wed, 20 Sep 2023 22:52:02 -0300 Subject: [PATCH 16/23] Adds a depply nested transaction test --- ...entTransactionUsingRefreshDatabaseTest.php | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/Integration/Database/EloquentTransactionUsingRefreshDatabaseTest.php b/tests/Integration/Database/EloquentTransactionUsingRefreshDatabaseTest.php index 8e030643db0c..2922f2b6b9cd 100644 --- a/tests/Integration/Database/EloquentTransactionUsingRefreshDatabaseTest.php +++ b/tests/Integration/Database/EloquentTransactionUsingRefreshDatabaseTest.php @@ -59,6 +59,30 @@ public function testObserverIsCalledOnTestsWithAfterCommitWhenUsingSavepointAndI $this->assertTrue($user1->exists); $this->assertEquals(1, $observer::$calledTimes, 'Failed to assert the observer was called once.'); } + + public function testObserverIsCalledEvenWhenDeeplyNestingTransactions() + { + User::observe($observer = EloquentTransactionUsingRefreshDatabaseUserObserver::resetting()); + + $user1 = DB::transaction(function () use ($observer) { + return tap(DB::transaction(function () use ($observer) { + return tap(DB::transaction(function () use ($observer) { + return tap(DB::transaction(function () { + return User::createOrFirst(UserFactory::new()->raw()); + }), function () use ($observer) { + $this->assertCount(0, $observer::$calledTimes, 'Should not have been called'); + }); + }), function () use ($observer) { + $this->assertCount(0, $observer::$calledTimes, 'Should not have been called'); + }); + }), function () use ($observer) { + $this->assertCount(0, $observer::$calledTimes, 'Should not have been called'); + }); + }); + + $this->assertTrue($user1->exists); + $this->assertEquals(1, $observer::$calledTimes, 'Failed to assert the observer was called once.'); + } } class EloquentTransactionUsingRefreshDatabaseUserObserver From d3dcb0364bf65d0f29b1226bb216c021b7475617 Mon Sep 17 00:00:00 2001 From: Tony Messias Date: Wed, 20 Sep 2023 23:00:00 -0300 Subject: [PATCH 17/23] Simplify deeply nesting test --- .../EloquentTransactionUsingRefreshDatabaseTest.php | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/tests/Integration/Database/EloquentTransactionUsingRefreshDatabaseTest.php b/tests/Integration/Database/EloquentTransactionUsingRefreshDatabaseTest.php index 2922f2b6b9cd..d167816534c2 100644 --- a/tests/Integration/Database/EloquentTransactionUsingRefreshDatabaseTest.php +++ b/tests/Integration/Database/EloquentTransactionUsingRefreshDatabaseTest.php @@ -66,17 +66,13 @@ public function testObserverIsCalledEvenWhenDeeplyNestingTransactions() $user1 = DB::transaction(function () use ($observer) { return tap(DB::transaction(function () use ($observer) { - return tap(DB::transaction(function () use ($observer) { - return tap(DB::transaction(function () { - return User::createOrFirst(UserFactory::new()->raw()); - }), function () use ($observer) { - $this->assertCount(0, $observer::$calledTimes, 'Should not have been called'); - }); + return tap(DB::transaction(function () { + return User::createOrFirst(UserFactory::new()->raw()); }), function () use ($observer) { - $this->assertCount(0, $observer::$calledTimes, 'Should not have been called'); + $this->assertEquals(0, $observer::$calledTimes, 'Should not have been called'); }); }), function () use ($observer) { - $this->assertCount(0, $observer::$calledTimes, 'Should not have been called'); + $this->assertEquals(0, $observer::$calledTimes, 'Should not have been called'); }); }); From 68ed29a8020fa1d6ddaa0132040c5b260a88045d Mon Sep 17 00:00:00 2001 From: Tony Messias Date: Wed, 20 Sep 2023 23:33:46 -0300 Subject: [PATCH 18/23] Sets default level value to one (since it's a new parameter) --- src/Illuminate/Database/DatabaseTransactionsManager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Illuminate/Database/DatabaseTransactionsManager.php b/src/Illuminate/Database/DatabaseTransactionsManager.php index cedea759557f..fd949cf0553e 100755 --- a/src/Illuminate/Database/DatabaseTransactionsManager.php +++ b/src/Illuminate/Database/DatabaseTransactionsManager.php @@ -76,7 +76,7 @@ public function rollback($connection, $level) * @param int $level * @return void */ - public function commit($connection, $level) + public function commit($connection, $level = 1) { // If the transaction level being commited reaches 1 (meaning it was the root // transaction), we'll run the callbacks. In test mode, since we wrap each From 5825929e54d06cd8a3ac1e2bb4e88b743f04bd89 Mon Sep 17 00:00:00 2001 From: Tony Messias Date: Fri, 22 Sep 2023 10:15:30 -0300 Subject: [PATCH 19/23] Rename method --- .../Database/DatabaseTransactionsManager.php | 12 ++++++------ .../Foundation/Testing/DatabaseTransactions.php | 2 +- .../Foundation/Testing/RefreshDatabase.php | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Illuminate/Database/DatabaseTransactionsManager.php b/src/Illuminate/Database/DatabaseTransactionsManager.php index fd949cf0553e..7886c6e5accb 100755 --- a/src/Illuminate/Database/DatabaseTransactionsManager.php +++ b/src/Illuminate/Database/DatabaseTransactionsManager.php @@ -16,7 +16,7 @@ class DatabaseTransactionsManager * * @var bool */ - protected $afterCommitCallbacksInTestMode = false; + protected $afterCommitCallbacksRunningInTestTransaction = false; /** * Create a new database transactions manager instance. @@ -26,7 +26,7 @@ class DatabaseTransactionsManager public function __construct() { $this->transactions = collect(); - $this->afterCommitCallbacksInTestMode = false; + $this->afterCommitCallbacksRunningInTestTransaction = false; } /** @@ -34,9 +34,9 @@ public function __construct() * * @return self */ - public function withAfterCommitCallbacksInTestMode() + public function withAfterCommitCallbacksInTestTransactionAwareMode() { - $this->afterCommitCallbacksInTestMode = true; + $this->afterCommitCallbacksRunningInTestTransaction = true; return $this; } @@ -81,7 +81,7 @@ public function commit($connection, $level = 1) // If the transaction level being commited reaches 1 (meaning it was the root // transaction), we'll run the callbacks. In test mode, since we wrap each // test in a transaction, we'll run the callbacks when reaching level 2. - if ($level == 1 || ($this->afterCommitCallbacksInTestMode && $level == 2)) { + if ($level == 1 || ($this->afterCommitCallbacksRunningInTestTransaction && $level == 2)) { [$forThisConnection, $forOtherConnections] = $this->transactions->partition( fn ($transaction) => $transaction->connection == $connection ); @@ -103,7 +103,7 @@ public function addCallback($callback) // If there are no transactions, we'll run the callbacks right away. Also, we'll run it // right away when we're in test mode and we only have the wrapping transaction. For // every other case, we'll queue up the callback to run after the commit happens. - if ($this->transactions->isEmpty() || ($this->afterCommitCallbacksInTestMode && $this->transactions->count() == 1)) { + if ($this->transactions->isEmpty() || ($this->afterCommitCallbacksRunningInTestTransaction && $this->transactions->count() == 1)) { return $callback(); } diff --git a/src/Illuminate/Foundation/Testing/DatabaseTransactions.php b/src/Illuminate/Foundation/Testing/DatabaseTransactions.php index 66cf4a54bef2..f9cab95fe62a 100644 --- a/src/Illuminate/Foundation/Testing/DatabaseTransactions.php +++ b/src/Illuminate/Foundation/Testing/DatabaseTransactions.php @@ -22,7 +22,7 @@ public function beginDatabaseTransaction() $connection->setEventDispatcher($dispatcher); if ($this->app->resolved('db.transactions')) { - $this->app->make('db.transactions')->withCallbacksExecutionInTestMode(); + $this->app->make('db.transactions')->withAfterCommitCallbacksInTestTransactionAwareMode(); } } diff --git a/src/Illuminate/Foundation/Testing/RefreshDatabase.php b/src/Illuminate/Foundation/Testing/RefreshDatabase.php index e04b3769697e..07a64dc98b2f 100644 --- a/src/Illuminate/Foundation/Testing/RefreshDatabase.php +++ b/src/Illuminate/Foundation/Testing/RefreshDatabase.php @@ -98,7 +98,7 @@ public function beginDatabaseTransaction() $connection->setEventDispatcher($dispatcher); if ($this->app->resolved('db.transactions')) { - $this->app->make('db.transactions')->withAfterCommitCallbacksInTestMode(); + $this->app->make('db.transactions')->withAfterCommitCallbacksInTestTransactionAwareMode(); } } From c66087986183391e7da4b17ac9c47d43490f0ccf Mon Sep 17 00:00:00 2001 From: Tony Messias Date: Fri, 22 Sep 2023 10:36:20 -0300 Subject: [PATCH 20/23] Adds back the removed methods from the db.transactions and mark them as deprecated --- .../Database/DatabaseTransactionsManager.php | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/Illuminate/Database/DatabaseTransactionsManager.php b/src/Illuminate/Database/DatabaseTransactionsManager.php index 7886c6e5accb..18dfee77e38b 100755 --- a/src/Illuminate/Database/DatabaseTransactionsManager.php +++ b/src/Illuminate/Database/DatabaseTransactionsManager.php @@ -110,6 +110,36 @@ public function addCallback($callback) return $this->transactions->last()->addCallback($callback); } + /** + * Specify that callbacks should ignore the given transaction when determining if they should be executed. + * + * @param \Illuminate\Database\DatabaseTransactionRecord $transaction + * @return $this + * + * @deprecated Will be removed in a future Laravel version. Use withAfterCommitCallbacksInTestTransactionAwareMode() instead. + */ + public function callbacksShouldIgnore(DatabaseTransactionRecord $transaction) + { + // This method was meant for testing only, so we're forwarding the call to the new method... + return $this->withAfterCommitCallbacksInTestTransactionAwareMode(); + } + + /** + * Get the transactions that are applicable to callbacks. + * + * @return \Illuminate\Support\Collection + * + * @deprecated Will be removed in a future Laravel version. + */ + public function callbackApplicableTransactions() + { + if (! $this->afterCommitCallbacksRunningInTestTransaction) { + return clone $this->transactions; + } + + return $this->transactions->skip(1)->values(); + } + /** * Get all the transactions. * From e111e70404ee1de8b84320c4cdfbeeb81441f1d3 Mon Sep 17 00:00:00 2001 From: Tony Messias Date: Fri, 22 Sep 2023 10:37:42 -0300 Subject: [PATCH 21/23] StyleCI --- src/Illuminate/Database/DatabaseTransactionsManager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Illuminate/Database/DatabaseTransactionsManager.php b/src/Illuminate/Database/DatabaseTransactionsManager.php index 18dfee77e38b..bbadac56c470 100755 --- a/src/Illuminate/Database/DatabaseTransactionsManager.php +++ b/src/Illuminate/Database/DatabaseTransactionsManager.php @@ -124,7 +124,7 @@ public function callbacksShouldIgnore(DatabaseTransactionRecord $transaction) return $this->withAfterCommitCallbacksInTestTransactionAwareMode(); } - /** + /** * Get the transactions that are applicable to callbacks. * * @return \Illuminate\Support\Collection From 239e6eca304ed02f96c626b34680fd89f24b1784 Mon Sep 17 00:00:00 2001 From: Tony Messias Date: Fri, 22 Sep 2023 10:42:40 -0300 Subject: [PATCH 22/23] Inline the if statement using then collection when() method --- src/Illuminate/Database/DatabaseTransactionsManager.php | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/Illuminate/Database/DatabaseTransactionsManager.php b/src/Illuminate/Database/DatabaseTransactionsManager.php index bbadac56c470..37797ad9e502 100755 --- a/src/Illuminate/Database/DatabaseTransactionsManager.php +++ b/src/Illuminate/Database/DatabaseTransactionsManager.php @@ -133,11 +133,9 @@ public function callbacksShouldIgnore(DatabaseTransactionRecord $transaction) */ public function callbackApplicableTransactions() { - if (! $this->afterCommitCallbacksRunningInTestTransaction) { - return clone $this->transactions; - } - - return $this->transactions->skip(1)->values(); + return $this->transactions + ->when($this->afterCommitCallbacksRunningInTestTransaction, fn ($transactions) => $transactions->skip(1)) + ->values(); } /** From 116f9de74cfef4013943fa3f1b90facb829b62e7 Mon Sep 17 00:00:00 2001 From: Mior Muhammad Zaki Date: Mon, 25 Sep 2023 10:26:45 +0800 Subject: [PATCH 23/23] wip --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index ce24f8536bd3..e062b0e03b8a 100644 --- a/composer.json +++ b/composer.json @@ -104,7 +104,7 @@ "league/flysystem-read-only": "^3.3", "league/flysystem-sftp-v3": "^3.0", "mockery/mockery": "^1.5.1", - "orchestra/testbench-core": "^8.10", + "orchestra/testbench-core": "^8.11.3", "pda/pheanstalk": "^4.0", "phpstan/phpstan": "^1.4.7", "phpunit/phpunit": "^10.0.7",