From 1f10b5e639b0537fc780d0c7d0302e8472792308 Mon Sep 17 00:00:00 2001 From: Tony Messias Date: Wed, 20 Sep 2023 00:56:29 -0300 Subject: [PATCH 01/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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 35109700aa2c3b2aedbfc6700cd2221237697044 Mon Sep 17 00:00:00 2001 From: Tony Messias Date: Mon, 25 Sep 2023 09:14:59 +0800 Subject: [PATCH 23/73] Tests observer using afterCommit Signed-off-by: Mior Muhammad Zaki --- ...entTransactionUsingRefreshDatabaseTest.php | 101 ++++++++++++++++++ 1 file changed, 101 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..d167816534c2 --- /dev/null +++ b/tests/Integration/Database/EloquentTransactionUsingRefreshDatabaseTest.php @@ -0,0 +1,101 @@ +afterApplicationCreated(fn () => User::unguard()); + $this->beforeApplicationDestroyed(fn () => User::reguard()); + + parent::setUp(); + } + + public function testObserverIsCalledOnTestsWithAfterCommit() + { + User::observe($observer = EloquentTransactionUsingRefreshDatabaseUserObserver::resetting()); + + $user1 = User::create(UserFactory::new()->raw()); + + $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(UserFactory::new()->raw())); + + $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(UserFactory::new()->raw()); + + $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(UserFactory::new()->raw())); + + $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 () { + return User::createOrFirst(UserFactory::new()->raw()); + }), function () use ($observer) { + $this->assertEquals(0, $observer::$calledTimes, 'Should not have been called'); + }); + }), function () use ($observer) { + $this->assertEquals(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 +{ + public static $calledTimes = 0; + + public $afterCommit = true; + + public static function resetting() + { + static::$calledTimes = 0; + + return new static(); + } + + public function created($user) + { + static::$calledTimes++; + } +} From 7bb02ae3d7301b265c2f5bb7ed341db024b18548 Mon Sep 17 00:00:00 2001 From: Mior Muhammad Zaki Date: Mon, 25 Sep 2023 09:28:17 +0800 Subject: [PATCH 24/73] wip Signed-off-by: Mior Muhammad Zaki --- .github/workflows/databases.yml | 38 +++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/.github/workflows/databases.yml b/.github/workflows/databases.yml index 20e008c66cbe..ab68503dd507 100644 --- a/.github/workflows/databases.yml +++ b/.github/workflows/databases.yml @@ -8,6 +8,44 @@ on: pull_request: jobs: + sqlite: + runs-on: ubuntu-22.04 + + strategy: + fail-fast: true + + name: SQLite + + steps: + - name: Checkout code + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Setup PHP + uses: shivammathur/setup-php@v2 + with: + php-version: 8.1 + extensions: dom, curl, libxml, mbstring, zip, pcntl, pdo, pdo_mysql + tools: composer:v2 + coverage: none + + - name: Install dependencies + uses: nick-fields/retry@v2 + with: + timeout_minutes: 5 + max_attempts: 5 + command: composer update --prefer-stable --prefer-dist --no-interaction --no-progress + + - name: Setup database + run: | + php vendor/bin/testbench package:create-sqlite-db + + - name: Execute tests + run: vendor/bin/phpunit tests/Integration/Database + env: + DB_CONNECTION: sqlite + mysql_57: runs-on: ubuntu-22.04 From 6799bde1a50f0f1639abdf9f0968c1daa2f9b61c Mon Sep 17 00:00:00 2001 From: Mior Muhammad Zaki Date: Mon, 25 Sep 2023 09:33:28 +0800 Subject: [PATCH 25/73] wip Signed-off-by: Mior Muhammad Zaki --- composer.json | 2 +- src/Illuminate/Database/DatabaseTransactionsManager.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/composer.json b/composer.json index ce24f8536bd3..e8377de5f574 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.2", "pda/pheanstalk": "^4.0", "phpstan/phpstan": "^1.4.7", "phpunit/phpunit": "^10.0.7", diff --git a/src/Illuminate/Database/DatabaseTransactionsManager.php b/src/Illuminate/Database/DatabaseTransactionsManager.php index 8d145188f065..cfa0e250c5fc 100755 --- a/src/Illuminate/Database/DatabaseTransactionsManager.php +++ b/src/Illuminate/Database/DatabaseTransactionsManager.php @@ -112,7 +112,7 @@ public function callbacksShouldIgnore(DatabaseTransactionRecord $transaction) /** * Get the transactions that are applicable to callbacks. * - * @return \Illuminate\Support\Collection + * @return \Illuminate\Support\Collection */ public function callbackApplicableTransactions() { From 9cc6900c107649fed9befc803bc5512da4dd5fc7 Mon Sep 17 00:00:00 2001 From: Mior Muhammad Zaki Date: Mon, 25 Sep 2023 09:42:27 +0800 Subject: [PATCH 26/73] wip Signed-off-by: Mior Muhammad Zaki --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index e8377de5f574..e2ed15808ef5 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.11.2", + "orchestra/testbench-core": "8.x-dev", "pda/pheanstalk": "^4.0", "phpstan/phpstan": "^1.4.7", "phpunit/phpunit": "^10.0.7", From ad9b10c7314d461ba4228b9d308da5be282133d9 Mon Sep 17 00:00:00 2001 From: Mior Muhammad Zaki Date: Mon, 25 Sep 2023 10:25:15 +0800 Subject: [PATCH 27/73] wip Signed-off-by: Mior Muhammad Zaki --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index e2ed15808ef5..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.x-dev", + "orchestra/testbench-core": "^8.11.3", "pda/pheanstalk": "^4.0", "phpstan/phpstan": "^1.4.7", "phpunit/phpunit": "^10.0.7", From 4a0232ba216e4d062d96f3698358390b89796742 Mon Sep 17 00:00:00 2001 From: Mior Muhammad Zaki Date: Mon, 25 Sep 2023 11:04:01 +0800 Subject: [PATCH 28/73] wip Signed-off-by: Mior Muhammad Zaki --- .../Database/DatabaseTransactionsManager.php | 14 +++++++++----- .../Foundation/Testing/DatabaseTransactions.php | 2 +- .../Foundation/Testing/RefreshDatabase.php | 2 +- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/Illuminate/Database/DatabaseTransactionsManager.php b/src/Illuminate/Database/DatabaseTransactionsManager.php index cfa0e250c5fc..ba0b9e9fe1cd 100755 --- a/src/Illuminate/Database/DatabaseTransactionsManager.php +++ b/src/Illuminate/Database/DatabaseTransactionsManager.php @@ -99,10 +99,10 @@ public function addCallback($callback) /** * Specify that callbacks should ignore the given transaction when determining if they should be executed. * - * @param \Illuminate\Database\DatabaseTransactionRecord $transaction + * @param \Illuminate\Database\DatabaseTransactionRecord|(callback(\Illuminate\Database\DatabaseTransactionRecord):bool) $transaction * @return $this */ - public function callbacksShouldIgnore(DatabaseTransactionRecord $transaction) + public function callbacksShouldIgnore($transaction) { $this->callbacksShouldIgnore = $transaction; @@ -116,9 +116,13 @@ public function callbacksShouldIgnore(DatabaseTransactionRecord $transaction) */ public function callbackApplicableTransactions() { - return $this->transactions->reject(function ($transaction) { - return $transaction === $this->callbacksShouldIgnore; - })->values(); + if (is_callable($this->callbacksShouldIgnore)) { + return $this->transactions->filter($this->callbacksShouldIgnore)->values(); + } + + return $this->transactions->reject( + fn ($transaction) => $transaction === $this->callbacksShouldIgnore + )->values(); } /** diff --git a/src/Illuminate/Foundation/Testing/DatabaseTransactions.php b/src/Illuminate/Foundation/Testing/DatabaseTransactions.php index d0e0bafc52de..cf01e3d65598 100644 --- a/src/Illuminate/Foundation/Testing/DatabaseTransactions.php +++ b/src/Illuminate/Foundation/Testing/DatabaseTransactions.php @@ -23,7 +23,7 @@ public function beginDatabaseTransaction() if ($this->app->resolved('db.transactions')) { $this->app->make('db.transactions')->callbacksShouldIgnore( - $this->app->make('db.transactions')->getTransactions()->first() + fn ($transaction) => $transaction === $this->app->make('db.transactions')->getTransactions()->first() ); } } diff --git a/src/Illuminate/Foundation/Testing/RefreshDatabase.php b/src/Illuminate/Foundation/Testing/RefreshDatabase.php index b486cbb0ac91..bd00520dcbd0 100644 --- a/src/Illuminate/Foundation/Testing/RefreshDatabase.php +++ b/src/Illuminate/Foundation/Testing/RefreshDatabase.php @@ -99,7 +99,7 @@ public function beginDatabaseTransaction() if ($this->app->resolved('db.transactions')) { $this->app->make('db.transactions')->callbacksShouldIgnore( - $this->app->make('db.transactions')->getTransactions()->first() + fn ($transaction) => $transaction === $this->app->make('db.transactions')->getTransactions()->first() ); } } From 36f1d21f792ba10253527c9aa9b9aef316e12b9a Mon Sep 17 00:00:00 2001 From: Mior Muhammad Zaki Date: Mon, 25 Sep 2023 11:16:06 +0800 Subject: [PATCH 29/73] wip Signed-off-by: Mior Muhammad Zaki --- src/Illuminate/Database/Concerns/ManagesTransactions.php | 2 +- src/Illuminate/Database/DatabaseTransactionsManager.php | 8 ++------ .../Foundation/Testing/DatabaseTransactions.php | 2 +- src/Illuminate/Foundation/Testing/RefreshDatabase.php | 2 +- 4 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/Illuminate/Database/Concerns/ManagesTransactions.php b/src/Illuminate/Database/Concerns/ManagesTransactions.php index 14661cc76ebf..b6ba89a08613 100644 --- a/src/Illuminate/Database/Concerns/ManagesTransactions.php +++ b/src/Illuminate/Database/Concerns/ManagesTransactions.php @@ -212,7 +212,7 @@ protected function afterCommitCallbacksShouldBeExecuted() { return $this->transactions == 0 || ($this->transactionsManager && - $this->transactionsManager->callbackApplicableTransactions()->count() === 1); + $this->transactionsManager->callbackApplicableTransactions()->count() >= 1); } /** diff --git a/src/Illuminate/Database/DatabaseTransactionsManager.php b/src/Illuminate/Database/DatabaseTransactionsManager.php index ba0b9e9fe1cd..ee35324bca80 100755 --- a/src/Illuminate/Database/DatabaseTransactionsManager.php +++ b/src/Illuminate/Database/DatabaseTransactionsManager.php @@ -99,10 +99,10 @@ public function addCallback($callback) /** * Specify that callbacks should ignore the given transaction when determining if they should be executed. * - * @param \Illuminate\Database\DatabaseTransactionRecord|(callback(\Illuminate\Database\DatabaseTransactionRecord):bool) $transaction + * @param \Illuminate\Database\DatabaseTransactionRecord $transaction * @return $this */ - public function callbacksShouldIgnore($transaction) + public function callbacksShouldIgnore(DatabaseTransactionRecord $transaction) { $this->callbacksShouldIgnore = $transaction; @@ -116,10 +116,6 @@ public function callbacksShouldIgnore($transaction) */ public function callbackApplicableTransactions() { - if (is_callable($this->callbacksShouldIgnore)) { - return $this->transactions->filter($this->callbacksShouldIgnore)->values(); - } - return $this->transactions->reject( fn ($transaction) => $transaction === $this->callbacksShouldIgnore )->values(); diff --git a/src/Illuminate/Foundation/Testing/DatabaseTransactions.php b/src/Illuminate/Foundation/Testing/DatabaseTransactions.php index cf01e3d65598..d0e0bafc52de 100644 --- a/src/Illuminate/Foundation/Testing/DatabaseTransactions.php +++ b/src/Illuminate/Foundation/Testing/DatabaseTransactions.php @@ -23,7 +23,7 @@ public function beginDatabaseTransaction() if ($this->app->resolved('db.transactions')) { $this->app->make('db.transactions')->callbacksShouldIgnore( - fn ($transaction) => $transaction === $this->app->make('db.transactions')->getTransactions()->first() + $this->app->make('db.transactions')->getTransactions()->first() ); } } diff --git a/src/Illuminate/Foundation/Testing/RefreshDatabase.php b/src/Illuminate/Foundation/Testing/RefreshDatabase.php index bd00520dcbd0..b486cbb0ac91 100644 --- a/src/Illuminate/Foundation/Testing/RefreshDatabase.php +++ b/src/Illuminate/Foundation/Testing/RefreshDatabase.php @@ -99,7 +99,7 @@ public function beginDatabaseTransaction() if ($this->app->resolved('db.transactions')) { $this->app->make('db.transactions')->callbacksShouldIgnore( - fn ($transaction) => $transaction === $this->app->make('db.transactions')->getTransactions()->first() + $this->app->make('db.transactions')->getTransactions()->first() ); } } From 2f3a94332eae539a2119e7df50b9e865f16eb5f0 Mon Sep 17 00:00:00 2001 From: Mior Muhammad Zaki Date: Mon, 25 Sep 2023 11:30:40 +0800 Subject: [PATCH 30/73] wip Signed-off-by: Mior Muhammad Zaki --- .../Database/Concerns/ManagesTransactions.php | 3 +- .../Database/DatabaseTransactionsManager.php | 35 +++++++++++++++++-- .../Testing/DatabaseTransactions.php | 2 ++ .../Foundation/Testing/RefreshDatabase.php | 2 ++ 4 files changed, 38 insertions(+), 4 deletions(-) diff --git a/src/Illuminate/Database/Concerns/ManagesTransactions.php b/src/Illuminate/Database/Concerns/ManagesTransactions.php index b6ba89a08613..c93a4665a19e 100644 --- a/src/Illuminate/Database/Concerns/ManagesTransactions.php +++ b/src/Illuminate/Database/Concerns/ManagesTransactions.php @@ -211,8 +211,7 @@ public function commit() protected function afterCommitCallbacksShouldBeExecuted() { return $this->transactions == 0 || - ($this->transactionsManager && - $this->transactionsManager->callbackApplicableTransactions()->count() >= 1); + ($this->transactionsManager?->afterCommitCallbacksShouldBeExecuted() ?? false); } /** diff --git a/src/Illuminate/Database/DatabaseTransactionsManager.php b/src/Illuminate/Database/DatabaseTransactionsManager.php index ee35324bca80..c6c2a1b22cbc 100755 --- a/src/Illuminate/Database/DatabaseTransactionsManager.php +++ b/src/Illuminate/Database/DatabaseTransactionsManager.php @@ -7,17 +7,24 @@ class DatabaseTransactionsManager /** * All of the recorded transactions. * - * @var \Illuminate\Support\Collection + * @var \Illuminate\Support\Collection */ protected $transactions; /** * The database transaction that should be ignored by callbacks. * - * @var \Illuminate\Database\DatabaseTransactionRecord + * @var \Illuminate\Database\DatabaseTransactionRecord|null */ protected $callbacksShouldIgnore; + /** + * The callback to determine after commit callback should be executed. + * + * @var (callable():bool)|null + */ + protected $afterCommitCallbacksShouldBeExecutedCallback; + /** * Create a new database transactions manager instance. * @@ -121,6 +128,30 @@ public function callbackApplicableTransactions() )->values(); } + /** + * Add custom callback to determine if after commit callbacks should be executed. + * + * @return bool + */ + public function afterCommitCallbacksShouldBeExecutedUsing(callable $callback = null) + { + $this->afterCommitCallbacksShouldBeExecutedCallback = $callback; + + return $this; + } + + /** + * Determine if after commit callbacks should be executed. + * + * @return bool + */ + public function afterCommitCallbacksShouldBeExecuted() + { + return is_callable($this->afterCommitCallbacksShouldBeExecutedCallback) + ? call_user_func($this->afterCommitCallbacksShouldBeExecutedCallback, $this->transactions) + : $this->callbackApplicableTransactions()->count() === 1; + } + /** * Get all the transactions. * diff --git a/src/Illuminate/Foundation/Testing/DatabaseTransactions.php b/src/Illuminate/Foundation/Testing/DatabaseTransactions.php index d0e0bafc52de..9ec69c2147ce 100644 --- a/src/Illuminate/Foundation/Testing/DatabaseTransactions.php +++ b/src/Illuminate/Foundation/Testing/DatabaseTransactions.php @@ -24,6 +24,8 @@ public function beginDatabaseTransaction() if ($this->app->resolved('db.transactions')) { $this->app->make('db.transactions')->callbacksShouldIgnore( $this->app->make('db.transactions')->getTransactions()->first() + )->afterCommitCallbacksShouldBeExecutedUsing( + fn($transactions) => $transactions->skip(1) ); } } diff --git a/src/Illuminate/Foundation/Testing/RefreshDatabase.php b/src/Illuminate/Foundation/Testing/RefreshDatabase.php index b486cbb0ac91..ea3262967330 100644 --- a/src/Illuminate/Foundation/Testing/RefreshDatabase.php +++ b/src/Illuminate/Foundation/Testing/RefreshDatabase.php @@ -100,6 +100,8 @@ public function beginDatabaseTransaction() if ($this->app->resolved('db.transactions')) { $this->app->make('db.transactions')->callbacksShouldIgnore( $this->app->make('db.transactions')->getTransactions()->first() + )->afterCommitCallbacksShouldBeExecutedUsing( + fn($transactions) => $transactions->skip(1) ); } } From 1e8bcf490f6d57bf021e2b25829632abc874b5f4 Mon Sep 17 00:00:00 2001 From: StyleCI Bot Date: Mon, 25 Sep 2023 03:31:07 +0000 Subject: [PATCH 31/73] Apply fixes from StyleCI --- src/Illuminate/Foundation/Testing/DatabaseTransactions.php | 2 +- src/Illuminate/Foundation/Testing/RefreshDatabase.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Illuminate/Foundation/Testing/DatabaseTransactions.php b/src/Illuminate/Foundation/Testing/DatabaseTransactions.php index 9ec69c2147ce..2b1e5a8c944e 100644 --- a/src/Illuminate/Foundation/Testing/DatabaseTransactions.php +++ b/src/Illuminate/Foundation/Testing/DatabaseTransactions.php @@ -25,7 +25,7 @@ public function beginDatabaseTransaction() $this->app->make('db.transactions')->callbacksShouldIgnore( $this->app->make('db.transactions')->getTransactions()->first() )->afterCommitCallbacksShouldBeExecutedUsing( - fn($transactions) => $transactions->skip(1) + fn ($transactions) => $transactions->skip(1) ); } } diff --git a/src/Illuminate/Foundation/Testing/RefreshDatabase.php b/src/Illuminate/Foundation/Testing/RefreshDatabase.php index ea3262967330..0d97d1a79e8e 100644 --- a/src/Illuminate/Foundation/Testing/RefreshDatabase.php +++ b/src/Illuminate/Foundation/Testing/RefreshDatabase.php @@ -101,7 +101,7 @@ public function beginDatabaseTransaction() $this->app->make('db.transactions')->callbacksShouldIgnore( $this->app->make('db.transactions')->getTransactions()->first() )->afterCommitCallbacksShouldBeExecutedUsing( - fn($transactions) => $transactions->skip(1) + fn ($transactions) => $transactions->skip(1) ); } } From f245912e9307e96b23e0b445178575a4fa6b003c Mon Sep 17 00:00:00 2001 From: Mior Muhammad Zaki Date: Mon, 25 Sep 2023 11:34:36 +0800 Subject: [PATCH 32/73] wip Signed-off-by: Mior Muhammad Zaki --- src/Illuminate/Foundation/Testing/RefreshDatabase.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Illuminate/Foundation/Testing/RefreshDatabase.php b/src/Illuminate/Foundation/Testing/RefreshDatabase.php index 0d97d1a79e8e..97471bae1d65 100644 --- a/src/Illuminate/Foundation/Testing/RefreshDatabase.php +++ b/src/Illuminate/Foundation/Testing/RefreshDatabase.php @@ -101,7 +101,7 @@ public function beginDatabaseTransaction() $this->app->make('db.transactions')->callbacksShouldIgnore( $this->app->make('db.transactions')->getTransactions()->first() )->afterCommitCallbacksShouldBeExecutedUsing( - fn ($transactions) => $transactions->skip(1) + fn ($transactions) => $transactions->skip(1) && $transactions->count() === 2 ); } } From be843efd99adbb687dcb6efeba036825a6408262 Mon Sep 17 00:00:00 2001 From: Mior Muhammad Zaki Date: Mon, 25 Sep 2023 11:38:06 +0800 Subject: [PATCH 33/73] wip Signed-off-by: Mior Muhammad Zaki --- src/Illuminate/Foundation/Testing/RefreshDatabase.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Illuminate/Foundation/Testing/RefreshDatabase.php b/src/Illuminate/Foundation/Testing/RefreshDatabase.php index 97471bae1d65..3d5eca715232 100644 --- a/src/Illuminate/Foundation/Testing/RefreshDatabase.php +++ b/src/Illuminate/Foundation/Testing/RefreshDatabase.php @@ -101,7 +101,7 @@ public function beginDatabaseTransaction() $this->app->make('db.transactions')->callbacksShouldIgnore( $this->app->make('db.transactions')->getTransactions()->first() )->afterCommitCallbacksShouldBeExecutedUsing( - fn ($transactions) => $transactions->skip(1) && $transactions->count() === 2 + fn ($transactions) => $transactions->skip(1)->count() === 1 ); } } From 6519d0d077e742ab04ecddfb306df7fabbc3775a Mon Sep 17 00:00:00 2001 From: Mior Muhammad Zaki Date: Mon, 25 Sep 2023 11:40:16 +0800 Subject: [PATCH 34/73] wip Signed-off-by: Mior Muhammad Zaki --- src/Illuminate/Foundation/Testing/RefreshDatabase.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/Illuminate/Foundation/Testing/RefreshDatabase.php b/src/Illuminate/Foundation/Testing/RefreshDatabase.php index 3d5eca715232..563c1c2aaf01 100644 --- a/src/Illuminate/Foundation/Testing/RefreshDatabase.php +++ b/src/Illuminate/Foundation/Testing/RefreshDatabase.php @@ -98,9 +98,11 @@ 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() - )->afterCommitCallbacksShouldBeExecutedUsing( + $this->app->make('db.transactions') + // ->callbacksShouldIgnore( + // $this->app->make('db.transactions')->getTransactions()->first() + // ) + ->afterCommitCallbacksShouldBeExecutedUsing( fn ($transactions) => $transactions->skip(1)->count() === 1 ); } From e2a079b695f749f104aa637ccf0c10f600406632 Mon Sep 17 00:00:00 2001 From: Mior Muhammad Zaki Date: Mon, 25 Sep 2023 12:11:02 +0800 Subject: [PATCH 35/73] wip Signed-off-by: Mior Muhammad Zaki --- src/Illuminate/Database/DatabaseTransactionsManager.php | 4 +++- src/Illuminate/Foundation/Testing/RefreshDatabase.php | 6 +++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/Illuminate/Database/DatabaseTransactionsManager.php b/src/Illuminate/Database/DatabaseTransactionsManager.php index c6c2a1b22cbc..1ded757f5350 100755 --- a/src/Illuminate/Database/DatabaseTransactionsManager.php +++ b/src/Illuminate/Database/DatabaseTransactionsManager.php @@ -96,7 +96,9 @@ public function commit($connection) */ public function addCallback($callback) { - if ($current = $this->callbackApplicableTransactions()->last()) { + $transactions = $this->callbackApplicableTransactions(); + + if ($transactions->count() >= 1 && $current = $this->callbackApplicableTransactions()->last()) { return $current->addCallback($callback); } diff --git a/src/Illuminate/Foundation/Testing/RefreshDatabase.php b/src/Illuminate/Foundation/Testing/RefreshDatabase.php index 563c1c2aaf01..4b597ebc19b1 100644 --- a/src/Illuminate/Foundation/Testing/RefreshDatabase.php +++ b/src/Illuminate/Foundation/Testing/RefreshDatabase.php @@ -99,9 +99,9 @@ public function beginDatabaseTransaction() if ($this->app->resolved('db.transactions')) { $this->app->make('db.transactions') - // ->callbacksShouldIgnore( - // $this->app->make('db.transactions')->getTransactions()->first() - // ) + ->callbacksShouldIgnore( + $this->app->make('db.transactions')->getTransactions()->first() + ) ->afterCommitCallbacksShouldBeExecutedUsing( fn ($transactions) => $transactions->skip(1)->count() === 1 ); From 21c6548a33193356872004c8b280ceb1db3fb08b Mon Sep 17 00:00:00 2001 From: Mior Muhammad Zaki Date: Mon, 25 Sep 2023 12:11:57 +0800 Subject: [PATCH 36/73] wip Signed-off-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 1ded757f5350..9b288d948724 100755 --- a/src/Illuminate/Database/DatabaseTransactionsManager.php +++ b/src/Illuminate/Database/DatabaseTransactionsManager.php @@ -98,7 +98,7 @@ public function addCallback($callback) { $transactions = $this->callbackApplicableTransactions(); - if ($transactions->count() >= 1 && $current = $this->callbackApplicableTransactions()->last()) { + if ($transactions->count() > 0 && $current = $this->callbackApplicableTransactions()->last()) { return $current->addCallback($callback); } From 5863f0cbb293899cc89e49b5d7f30fb61cd7f145 Mon Sep 17 00:00:00 2001 From: Mior Muhammad Zaki Date: Mon, 25 Sep 2023 12:15:35 +0800 Subject: [PATCH 37/73] wip Signed-off-by: Mior Muhammad Zaki --- src/Illuminate/Foundation/Testing/RefreshDatabase.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Illuminate/Foundation/Testing/RefreshDatabase.php b/src/Illuminate/Foundation/Testing/RefreshDatabase.php index 4b597ebc19b1..3c4a1068da8c 100644 --- a/src/Illuminate/Foundation/Testing/RefreshDatabase.php +++ b/src/Illuminate/Foundation/Testing/RefreshDatabase.php @@ -101,9 +101,6 @@ public function beginDatabaseTransaction() $this->app->make('db.transactions') ->callbacksShouldIgnore( $this->app->make('db.transactions')->getTransactions()->first() - ) - ->afterCommitCallbacksShouldBeExecutedUsing( - fn ($transactions) => $transactions->skip(1)->count() === 1 ); } } From 75713186c8004b559645ff705302974af546bcad Mon Sep 17 00:00:00 2001 From: Mior Muhammad Zaki Date: Mon, 25 Sep 2023 12:20:58 +0800 Subject: [PATCH 38/73] wip Signed-off-by: Mior Muhammad Zaki --- src/Illuminate/Foundation/Testing/RefreshDatabase.php | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/Illuminate/Foundation/Testing/RefreshDatabase.php b/src/Illuminate/Foundation/Testing/RefreshDatabase.php index 3c4a1068da8c..84283ddc7cd0 100644 --- a/src/Illuminate/Foundation/Testing/RefreshDatabase.php +++ b/src/Illuminate/Foundation/Testing/RefreshDatabase.php @@ -99,9 +99,12 @@ public function beginDatabaseTransaction() if ($this->app->resolved('db.transactions')) { $this->app->make('db.transactions') - ->callbacksShouldIgnore( - $this->app->make('db.transactions')->getTransactions()->first() - ); + ->callbacksShouldIgnore( + $this->app->make('db.transactions')->getTransactions()->first() + ) + ->afterCommitCallbacksShouldBeExecutedUsing( + fn ($transactions) => $transactions->skip(1)->count() <= 1 + ); } } From a8882f5199cb33649df90e4617903b166cf3f1bb Mon Sep 17 00:00:00 2001 From: Mior Muhammad Zaki Date: Mon, 25 Sep 2023 13:05:02 +0800 Subject: [PATCH 39/73] wip Signed-off-by: Mior Muhammad Zaki --- .../Database/Concerns/ManagesTransactions.php | 4 ---- src/Illuminate/Database/Connection.php | 10 ++++++++++ .../Database/DatabaseTransactionsManager.php | 8 ++++---- .../Foundation/Testing/RefreshDatabase.php | 14 ++++++-------- ...quentTransactionUsingRefreshDatabaseTest.php | 17 ++++++++++++----- 5 files changed, 32 insertions(+), 21 deletions(-) diff --git a/src/Illuminate/Database/Concerns/ManagesTransactions.php b/src/Illuminate/Database/Concerns/ManagesTransactions.php index c93a4665a19e..551a9e94e94e 100644 --- a/src/Illuminate/Database/Concerns/ManagesTransactions.php +++ b/src/Illuminate/Database/Concerns/ManagesTransactions.php @@ -47,8 +47,6 @@ 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()); } @@ -194,8 +192,6 @@ public function commit() $this->getPdo()->commit(); } - $this->transactions = max(0, $this->transactions - 1); - if ($this->afterCommitCallbacksShouldBeExecuted()) { $this->transactionsManager?->commit($this->getName()); } diff --git a/src/Illuminate/Database/Connection.php b/src/Illuminate/Database/Connection.php index eb14815b87b9..a07a70ce843e 100755 --- a/src/Illuminate/Database/Connection.php +++ b/src/Illuminate/Database/Connection.php @@ -1522,6 +1522,16 @@ public function unsetEventDispatcher() $this->events = null; } + /** + * Get the transaction manager instance on the connection. + * + * @return \Illuminate\Database\DatabaseTransactionsManager + */ + public function getTransactionManager() + { + return $this->transactionsManager; + } + /** * Set the transaction manager instance on the connection. * diff --git a/src/Illuminate/Database/DatabaseTransactionsManager.php b/src/Illuminate/Database/DatabaseTransactionsManager.php index 9b288d948724..7cfbca569954 100755 --- a/src/Illuminate/Database/DatabaseTransactionsManager.php +++ b/src/Illuminate/Database/DatabaseTransactionsManager.php @@ -98,11 +98,11 @@ public function addCallback($callback) { $transactions = $this->callbackApplicableTransactions(); - if ($transactions->count() > 0 && $current = $this->callbackApplicableTransactions()->last()) { - return $current->addCallback($callback); + if ($transactions->isEmpty()) { + $callback(); + } elseif ($current = $transactions->first()) { + $current->addCallback($callback); } - - $callback(); } /** diff --git a/src/Illuminate/Foundation/Testing/RefreshDatabase.php b/src/Illuminate/Foundation/Testing/RefreshDatabase.php index 84283ddc7cd0..39918866c0ce 100644 --- a/src/Illuminate/Foundation/Testing/RefreshDatabase.php +++ b/src/Illuminate/Foundation/Testing/RefreshDatabase.php @@ -97,14 +97,12 @@ public function beginDatabaseTransaction() $connection->beginTransaction(); $connection->setEventDispatcher($dispatcher); - if ($this->app->resolved('db.transactions')) { - $this->app->make('db.transactions') - ->callbacksShouldIgnore( - $this->app->make('db.transactions')->getTransactions()->first() - ) - ->afterCommitCallbacksShouldBeExecutedUsing( - fn ($transactions) => $transactions->skip(1)->count() <= 1 - ); + if ($transactionManager = $connection->getTransactionManager()) { + $transactionManager->callbacksShouldIgnore( + $transactionManager->getTransactions()->first() + )->afterCommitCallbacksShouldBeExecutedUsing(function ($transactions) { + return true; + }); } } diff --git a/tests/Integration/Database/EloquentTransactionUsingRefreshDatabaseTest.php b/tests/Integration/Database/EloquentTransactionUsingRefreshDatabaseTest.php index d167816534c2..3f696dca7ee5 100644 --- a/tests/Integration/Database/EloquentTransactionUsingRefreshDatabaseTest.php +++ b/tests/Integration/Database/EloquentTransactionUsingRefreshDatabaseTest.php @@ -20,6 +20,13 @@ protected function setUp(): void parent::setUp(); } + protected function getEnvironmentSetUp($app) + { + $app['config']->set('database.default', 'sqlite'); + + parent::getEnvironmentSetUp($app); + } + public function testObserverIsCalledOnTestsWithAfterCommit() { User::observe($observer = EloquentTransactionUsingRefreshDatabaseUserObserver::resetting()); @@ -66,13 +73,13 @@ public function testObserverIsCalledEvenWhenDeeplyNestingTransactions() $user1 = 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->assertEquals(0, $observer::$calledTimes, 'Should not have been called'); + return DB::transaction(function () use ($observer) { + return tap(User::createOrFirst(UserFactory::new()->raw()), function () use ($observer) { + //$this->assertEquals(0, $observer::$calledTimes, 'Should not have been called'); + }); }); }), function () use ($observer) { - $this->assertEquals(0, $observer::$calledTimes, 'Should not have been called'); + // $this->assertEquals(0, $observer::$calledTimes, 'Should not have been called'); }); }); From d4ec4a2ec01eafc6aa87853092d9b489a55a31d6 Mon Sep 17 00:00:00 2001 From: StyleCI Bot Date: Mon, 25 Sep 2023 05:05:18 +0000 Subject: [PATCH 40/73] Apply fixes from StyleCI --- src/Illuminate/Database/Connection.php | 2 +- .../EloquentTransactionUsingRefreshDatabaseTest.php | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Illuminate/Database/Connection.php b/src/Illuminate/Database/Connection.php index a07a70ce843e..30e6940af5ed 100755 --- a/src/Illuminate/Database/Connection.php +++ b/src/Illuminate/Database/Connection.php @@ -1525,7 +1525,7 @@ public function unsetEventDispatcher() /** * Get the transaction manager instance on the connection. * - * @return \Illuminate\Database\DatabaseTransactionsManager + * @return \Illuminate\Database\DatabaseTransactionsManager */ public function getTransactionManager() { diff --git a/tests/Integration/Database/EloquentTransactionUsingRefreshDatabaseTest.php b/tests/Integration/Database/EloquentTransactionUsingRefreshDatabaseTest.php index 3f696dca7ee5..39c34179ae58 100644 --- a/tests/Integration/Database/EloquentTransactionUsingRefreshDatabaseTest.php +++ b/tests/Integration/Database/EloquentTransactionUsingRefreshDatabaseTest.php @@ -71,14 +71,14 @@ public function testObserverIsCalledEvenWhenDeeplyNestingTransactions() { User::observe($observer = EloquentTransactionUsingRefreshDatabaseUserObserver::resetting()); - $user1 = DB::transaction(function () use ($observer) { - return tap(DB::transaction(function () use ($observer) { - return DB::transaction(function () use ($observer) { - return tap(User::createOrFirst(UserFactory::new()->raw()), function () use ($observer) { + $user1 = DB::transaction(function () { + return tap(DB::transaction(function () { + return DB::transaction(function () { + return tap(User::createOrFirst(UserFactory::new()->raw()), function () { //$this->assertEquals(0, $observer::$calledTimes, 'Should not have been called'); }); }); - }), function () use ($observer) { + }), function () { // $this->assertEquals(0, $observer::$calledTimes, 'Should not have been called'); }); }); From 8e9eae3521833677ef03da4078ecd2a767e84db2 Mon Sep 17 00:00:00 2001 From: Mior Muhammad Zaki Date: Mon, 25 Sep 2023 13:06:22 +0800 Subject: [PATCH 41/73] wip Signed-off-by: Mior Muhammad Zaki --- .../EloquentTransactionUsingRefreshDatabaseTest.php | 7 ------- 1 file changed, 7 deletions(-) diff --git a/tests/Integration/Database/EloquentTransactionUsingRefreshDatabaseTest.php b/tests/Integration/Database/EloquentTransactionUsingRefreshDatabaseTest.php index 3f696dca7ee5..3cd3bc171534 100644 --- a/tests/Integration/Database/EloquentTransactionUsingRefreshDatabaseTest.php +++ b/tests/Integration/Database/EloquentTransactionUsingRefreshDatabaseTest.php @@ -20,13 +20,6 @@ protected function setUp(): void parent::setUp(); } - protected function getEnvironmentSetUp($app) - { - $app['config']->set('database.default', 'sqlite'); - - parent::getEnvironmentSetUp($app); - } - public function testObserverIsCalledOnTestsWithAfterCommit() { User::observe($observer = EloquentTransactionUsingRefreshDatabaseUserObserver::resetting()); From 9968ad2b46e10fbee082881ad31d63923132b9a2 Mon Sep 17 00:00:00 2001 From: Mior Muhammad Zaki Date: Mon, 25 Sep 2023 14:08:53 +0800 Subject: [PATCH 42/73] wip Signed-off-by: Mior Muhammad Zaki --- src/Illuminate/Database/DatabaseTransactionRecord.php | 2 ++ .../Database/DatabaseTransactionsManager.php | 6 ++---- src/Illuminate/Foundation/Testing/RefreshDatabase.php | 10 +++++++--- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/Illuminate/Database/DatabaseTransactionRecord.php b/src/Illuminate/Database/DatabaseTransactionRecord.php index 4736ee9224d2..3d90fbd6ee8c 100755 --- a/src/Illuminate/Database/DatabaseTransactionRecord.php +++ b/src/Illuminate/Database/DatabaseTransactionRecord.php @@ -59,6 +59,8 @@ public function executeCallbacks() foreach ($this->callbacks as $callback) { $callback(); } + + $this->callbacks = []; } /** diff --git a/src/Illuminate/Database/DatabaseTransactionsManager.php b/src/Illuminate/Database/DatabaseTransactionsManager.php index 7cfbca569954..6772377f70ab 100755 --- a/src/Illuminate/Database/DatabaseTransactionsManager.php +++ b/src/Illuminate/Database/DatabaseTransactionsManager.php @@ -100,7 +100,7 @@ public function addCallback($callback) if ($transactions->isEmpty()) { $callback(); - } elseif ($current = $transactions->first()) { + } elseif ($current = $transactions->last()) { $current->addCallback($callback); } } @@ -149,9 +149,7 @@ public function afterCommitCallbacksShouldBeExecutedUsing(callable $callback = n */ public function afterCommitCallbacksShouldBeExecuted() { - return is_callable($this->afterCommitCallbacksShouldBeExecutedCallback) - ? call_user_func($this->afterCommitCallbacksShouldBeExecutedCallback, $this->transactions) - : $this->callbackApplicableTransactions()->count() === 1; + return $this->callbackApplicableTransactions()->count() === 1; } /** diff --git a/src/Illuminate/Foundation/Testing/RefreshDatabase.php b/src/Illuminate/Foundation/Testing/RefreshDatabase.php index 39918866c0ce..40d276505c85 100644 --- a/src/Illuminate/Foundation/Testing/RefreshDatabase.php +++ b/src/Illuminate/Foundation/Testing/RefreshDatabase.php @@ -100,9 +100,7 @@ public function beginDatabaseTransaction() if ($transactionManager = $connection->getTransactionManager()) { $transactionManager->callbacksShouldIgnore( $transactionManager->getTransactions()->first() - )->afterCommitCallbacksShouldBeExecutedUsing(function ($transactions) { - return true; - }); + ); } } @@ -111,6 +109,12 @@ public function beginDatabaseTransaction() $connection = $database->connection($name); $dispatcher = $connection->getEventDispatcher(); + if ($transactionManager = $connection->getTransactionManager()) { + $transactionManager->getTransactions() + ->skip(1) + ->map->executeCallbacks(); + } + $connection->unsetEventDispatcher(); $connection->rollBack(); $connection->setEventDispatcher($dispatcher); From b3c3951f282dfc750e304cb7ee2d6f2cd2cbeffb Mon Sep 17 00:00:00 2001 From: Mior Muhammad Zaki Date: Mon, 25 Sep 2023 14:12:16 +0800 Subject: [PATCH 43/73] wip Signed-off-by: Mior Muhammad Zaki --- .../Database/Concerns/ManagesTransactions.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Illuminate/Database/Concerns/ManagesTransactions.php b/src/Illuminate/Database/Concerns/ManagesTransactions.php index 551a9e94e94e..292c9159788e 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(); } - if ($this->afterCommitCallbacksShouldBeExecuted()) { - $this->transactionsManager?->commit($this->getName()); - } + $this->transactionsManager?->commit($this->getName(), $this->transactions); + + $this->transactions = max(0, $this->transactions - 1); } catch (Throwable $e) { $this->handleCommitTransactionException( $e, $currentAttempt, $attempts @@ -192,9 +192,9 @@ public function commit() $this->getPdo()->commit(); } - if ($this->afterCommitCallbacksShouldBeExecuted()) { - $this->transactionsManager?->commit($this->getName()); - } + $this->transactionsManager?->commit($this->getName(), $this->transactions); + + $this->transactions = max(0, $this->transactions - 1); $this->fireConnectionEvent('committed'); } From d13db216adda29a722aa900b81ba9cd733ef57bc Mon Sep 17 00:00:00 2001 From: Mior Muhammad Zaki Date: Mon, 25 Sep 2023 14:20:10 +0800 Subject: [PATCH 44/73] wip Signed-off-by: Mior Muhammad Zaki --- .../Database/Concerns/ManagesTransactions.php | 4 +-- .../Database/DatabaseTransactionsManager.php | 35 ++++++++++++------- .../Foundation/Testing/RefreshDatabase.php | 2 +- 3 files changed, 26 insertions(+), 15 deletions(-) diff --git a/src/Illuminate/Database/Concerns/ManagesTransactions.php b/src/Illuminate/Database/Concerns/ManagesTransactions.php index 292c9159788e..9b70f3e6fa90 100644 --- a/src/Illuminate/Database/Concerns/ManagesTransactions.php +++ b/src/Illuminate/Database/Concerns/ManagesTransactions.php @@ -48,14 +48,14 @@ public function transaction(Closure $callback, $attempts = 1) } $this->transactionsManager?->commit($this->getName(), $this->transactions); - - $this->transactions = max(0, $this->transactions - 1); } catch (Throwable $e) { $this->handleCommitTransactionException( $e, $currentAttempt, $attempts ); continue; + } finally { + $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 6772377f70ab..6352574e5cc5 100755 --- a/src/Illuminate/Database/DatabaseTransactionsManager.php +++ b/src/Illuminate/Database/DatabaseTransactionsManager.php @@ -71,20 +71,26 @@ public function rollback($connection, $level) * Commit the active database transaction. * * @param string $connection + * @param int $level * @return void */ - public function commit($connection) + public function commit($connection, $level = 1) { [$forThisConnection, $forOtherConnections] = $this->transactions->partition( fn ($transaction) => $transaction->connection == $connection ); - $this->transactions = $forOtherConnections->values(); + // 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 ($this->afterCommitCallbacksShouldBeExecuted($level)) { + $this->transactions = $forOtherConnections->values(); - $forThisConnection->map->executeCallbacks(); + $forThisConnection->map->executeCallbacks(); - if ($this->transactions->isEmpty()) { - $this->callbacksShouldIgnore = null; + if ($this->transactions->isEmpty()) { + $this->callbacksShouldIgnore = null; + } } } @@ -96,12 +102,13 @@ public function commit($connection) */ public function addCallback($callback) { - $transactions = $this->callbackApplicableTransactions(); - - if ($transactions->isEmpty()) { + // 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->callbackApplicableTransactions()->count() == 1)) { $callback(); - } elseif ($current = $transactions->last()) { - $current->addCallback($callback); + } else { + $this->transactions->last()->addCallback($callback); } } @@ -133,6 +140,7 @@ public function callbackApplicableTransactions() /** * Add custom callback to determine if after commit callbacks should be executed. * + * @param (callable(int, \Illuminate\Support\Collection):(bool))|null $callback * @return bool */ public function afterCommitCallbacksShouldBeExecutedUsing(callable $callback = null) @@ -145,11 +153,14 @@ public function afterCommitCallbacksShouldBeExecutedUsing(callable $callback = n /** * Determine if after commit callbacks should be executed. * + * @param int $level * @return bool */ - public function afterCommitCallbacksShouldBeExecuted() + public function afterCommitCallbacksShouldBeExecuted($level) { - return $this->callbackApplicableTransactions()->count() === 1; + return is_callable($this->afterCommitCallbacksShouldBeExecutedCallback) + ? call_user_func($this->afterCommitCallbacksShouldBeExecutedCallback, $level, $this->transactions) + : $level === 1; } /** diff --git a/src/Illuminate/Foundation/Testing/RefreshDatabase.php b/src/Illuminate/Foundation/Testing/RefreshDatabase.php index 40d276505c85..e53852156b77 100644 --- a/src/Illuminate/Foundation/Testing/RefreshDatabase.php +++ b/src/Illuminate/Foundation/Testing/RefreshDatabase.php @@ -100,7 +100,7 @@ public function beginDatabaseTransaction() if ($transactionManager = $connection->getTransactionManager()) { $transactionManager->callbacksShouldIgnore( $transactionManager->getTransactions()->first() - ); + )->afterCommitCallbacksShouldBeExecuted(fn ($level) => $level === 2); } } From e5993332f5db51cba3f7e892f3ea6f4a197e635a Mon Sep 17 00:00:00 2001 From: Mior Muhammad Zaki Date: Mon, 25 Sep 2023 14:25:57 +0800 Subject: [PATCH 45/73] wip Signed-off-by: Mior Muhammad Zaki --- src/Illuminate/Database/Concerns/ManagesTransactions.php | 4 +++- src/Illuminate/Database/DatabaseTransactionsManager.php | 2 +- src/Illuminate/Foundation/Testing/RefreshDatabase.php | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/Illuminate/Database/Concerns/ManagesTransactions.php b/src/Illuminate/Database/Concerns/ManagesTransactions.php index 9b70f3e6fa90..11b0b22ad9c4 100644 --- a/src/Illuminate/Database/Concerns/ManagesTransactions.php +++ b/src/Illuminate/Database/Concerns/ManagesTransactions.php @@ -203,11 +203,13 @@ public function commit() * Determine if after commit callbacks should be executed. * * @return bool + * + * @deprecated */ protected function afterCommitCallbacksShouldBeExecuted() { return $this->transactions == 0 || - ($this->transactionsManager?->afterCommitCallbacksShouldBeExecuted() ?? false); + ($this->transactionsManager?->afterCommitCallbacksShouldBeExecuted(1)); } /** diff --git a/src/Illuminate/Database/DatabaseTransactionsManager.php b/src/Illuminate/Database/DatabaseTransactionsManager.php index 6352574e5cc5..c9f2f5ab71de 100755 --- a/src/Illuminate/Database/DatabaseTransactionsManager.php +++ b/src/Illuminate/Database/DatabaseTransactionsManager.php @@ -105,7 +105,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->callbackApplicableTransactions()->count() == 1)) { + if ($this->callbackApplicableTransactions()->count() === 0) { $callback(); } else { $this->transactions->last()->addCallback($callback); diff --git a/src/Illuminate/Foundation/Testing/RefreshDatabase.php b/src/Illuminate/Foundation/Testing/RefreshDatabase.php index e53852156b77..9f01ea54cf8e 100644 --- a/src/Illuminate/Foundation/Testing/RefreshDatabase.php +++ b/src/Illuminate/Foundation/Testing/RefreshDatabase.php @@ -100,7 +100,7 @@ public function beginDatabaseTransaction() if ($transactionManager = $connection->getTransactionManager()) { $transactionManager->callbacksShouldIgnore( $transactionManager->getTransactions()->first() - )->afterCommitCallbacksShouldBeExecuted(fn ($level) => $level === 2); + )->afterCommitCallbacksShouldBeExecutedUsing(fn ($level) => $level === 2); } } From 7c99f62120a8edfa1c453b7be28c991ac4934007 Mon Sep 17 00:00:00 2001 From: StyleCI Bot Date: Mon, 25 Sep 2023 06:26:12 +0000 Subject: [PATCH 46/73] Apply fixes from StyleCI --- src/Illuminate/Database/Concerns/ManagesTransactions.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Illuminate/Database/Concerns/ManagesTransactions.php b/src/Illuminate/Database/Concerns/ManagesTransactions.php index 11b0b22ad9c4..6b7ee86a68a3 100644 --- a/src/Illuminate/Database/Concerns/ManagesTransactions.php +++ b/src/Illuminate/Database/Concerns/ManagesTransactions.php @@ -209,7 +209,7 @@ public function commit() protected function afterCommitCallbacksShouldBeExecuted() { return $this->transactions == 0 || - ($this->transactionsManager?->afterCommitCallbacksShouldBeExecuted(1)); + $this->transactionsManager?->afterCommitCallbacksShouldBeExecuted(1); } /** From c645396583aa2351eb31ac63578e8d27cf4c2885 Mon Sep 17 00:00:00 2001 From: Mior Muhammad Zaki Date: Mon, 25 Sep 2023 14:35:05 +0800 Subject: [PATCH 47/73] wip Signed-off-by: Mior Muhammad Zaki --- .../Testing/DatabaseTransactions.php | 10 +++---- ...oquentTransactionWithAfterCommitTests.php} | 27 ++++++++++--------- ...terCommitUsingDatabaseTransactionsTest.php | 11 ++++++++ ...ithAfterCommitUsingRefreshDatabaseTest.php | 11 ++++++++ 4 files changed, 40 insertions(+), 19 deletions(-) rename tests/Integration/Database/{EloquentTransactionUsingRefreshDatabaseTest.php => EloquentTransactionWithAfterCommitTests.php} (73%) create mode 100644 tests/Integration/Database/EloquentTransactionWithAfterCommitUsingDatabaseTransactionsTest.php create mode 100644 tests/Integration/Database/EloquentTransactionWithAfterCommitUsingRefreshDatabaseTest.php diff --git a/src/Illuminate/Foundation/Testing/DatabaseTransactions.php b/src/Illuminate/Foundation/Testing/DatabaseTransactions.php index 2b1e5a8c944e..8a679f036c3d 100644 --- a/src/Illuminate/Foundation/Testing/DatabaseTransactions.php +++ b/src/Illuminate/Foundation/Testing/DatabaseTransactions.php @@ -21,12 +21,10 @@ public function beginDatabaseTransaction() $connection->beginTransaction(); $connection->setEventDispatcher($dispatcher); - if ($this->app->resolved('db.transactions')) { - $this->app->make('db.transactions')->callbacksShouldIgnore( - $this->app->make('db.transactions')->getTransactions()->first() - )->afterCommitCallbacksShouldBeExecutedUsing( - fn ($transactions) => $transactions->skip(1) - ); + if ($transactionManager = $connection->getTransactionManager()) { + $transactionManager->callbacksShouldIgnore( + $transactionManager->getTransactions()->first() + )->afterCommitCallbacksShouldBeExecutedUsing(fn ($level) => $level === 2); } } diff --git a/tests/Integration/Database/EloquentTransactionUsingRefreshDatabaseTest.php b/tests/Integration/Database/EloquentTransactionWithAfterCommitTests.php similarity index 73% rename from tests/Integration/Database/EloquentTransactionUsingRefreshDatabaseTest.php rename to tests/Integration/Database/EloquentTransactionWithAfterCommitTests.php index 6fede6dce23a..42fc1724c423 100644 --- a/tests/Integration/Database/EloquentTransactionUsingRefreshDatabaseTest.php +++ b/tests/Integration/Database/EloquentTransactionWithAfterCommitTests.php @@ -3,26 +3,27 @@ namespace Illuminate\Tests\Integration\Database; use Illuminate\Foundation\Auth\User; -use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Facades\DB; use Orchestra\Testbench\Concerns\WithLaravelMigrations; use Orchestra\Testbench\Factories\UserFactory; -class EloquentTransactionUsingRefreshDatabaseTest extends DatabaseTestCase +trait EloquentTransactionWithAfterCommitTests { - use RefreshDatabase, WithLaravelMigrations; + use WithLaravelMigrations; - protected function setUp(): void + protected function setUpEloquentTransactionWithAfterCommitTests(): void { - $this->afterApplicationCreated(fn () => User::unguard()); - $this->beforeApplicationDestroyed(fn () => User::reguard()); + User::unguard(); + } - parent::setUp(); + protected function tearDownEloquentTransactionWithAfterCommitTests(): void + { + User::reguard(); } public function testObserverIsCalledOnTestsWithAfterCommit() { - User::observe($observer = EloquentTransactionUsingRefreshDatabaseUserObserver::resetting()); + User::observe($observer = EloquentTransactionWithAfterCommitTestsUserObserver::resetting()); $user1 = User::create(UserFactory::new()->raw()); @@ -32,7 +33,7 @@ public function testObserverIsCalledOnTestsWithAfterCommit() public function testObserverCalledWithAfterCommitWhenInsideTransaction() { - User::observe($observer = EloquentTransactionUsingRefreshDatabaseUserObserver::resetting()); + User::observe($observer = EloquentTransactionWithAfterCommitTestsUserObserver::resetting()); $user1 = DB::transaction(fn () => User::create(UserFactory::new()->raw())); @@ -42,7 +43,7 @@ public function testObserverCalledWithAfterCommitWhenInsideTransaction() public function testObserverIsCalledOnTestsWithAfterCommitWhenUsingSavepoint() { - User::observe($observer = EloquentTransactionUsingRefreshDatabaseUserObserver::resetting()); + User::observe($observer = EloquentTransactionWithAfterCommitTestsUserObserver::resetting()); $user1 = User::createOrFirst(UserFactory::new()->raw()); @@ -52,7 +53,7 @@ public function testObserverIsCalledOnTestsWithAfterCommitWhenUsingSavepoint() public function testObserverIsCalledOnTestsWithAfterCommitWhenUsingSavepointAndInsideTransaction() { - User::observe($observer = EloquentTransactionUsingRefreshDatabaseUserObserver::resetting()); + User::observe($observer = EloquentTransactionWithAfterCommitTestsUserObserver::resetting()); $user1 = DB::transaction(fn () => User::createOrFirst(UserFactory::new()->raw())); @@ -62,7 +63,7 @@ public function testObserverIsCalledOnTestsWithAfterCommitWhenUsingSavepointAndI public function testObserverIsCalledEvenWhenDeeplyNestingTransactions() { - User::observe($observer = EloquentTransactionUsingRefreshDatabaseUserObserver::resetting()); + User::observe($observer = EloquentTransactionWithAfterCommitTestsUserObserver::resetting()); $user1 = DB::transaction(function () { return tap(DB::transaction(function () { @@ -81,7 +82,7 @@ public function testObserverIsCalledEvenWhenDeeplyNestingTransactions() } } -class EloquentTransactionUsingRefreshDatabaseUserObserver +class EloquentTransactionWithAfterCommitTestsUserObserver { public static $calledTimes = 0; diff --git a/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingDatabaseTransactionsTest.php b/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingDatabaseTransactionsTest.php new file mode 100644 index 000000000000..bf85cfa652a1 --- /dev/null +++ b/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingDatabaseTransactionsTest.php @@ -0,0 +1,11 @@ + Date: Mon, 25 Sep 2023 14:38:22 +0800 Subject: [PATCH 48/73] wip Signed-off-by: Mior Muhammad Zaki --- phpunit.xml.dist | 2 +- ...baseTest.php => EloquentTransactionWithAfterCommitTest.php} | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) rename tests/Integration/Database/{EloquentTransactionWithAfterCommitUsingRefreshDatabaseTest.php => EloquentTransactionWithAfterCommitTest.php} (58%) diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 7265cc649940..5ff4878d8771 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -16,7 +16,7 @@ - +