From 0870ac6b797a1ad10ccbf0c67ea1c2dded5a2393 Mon Sep 17 00:00:00 2001 From: Tony Messias Date: Fri, 1 Nov 2024 17:31:25 -0300 Subject: [PATCH 1/9] Skip the number of connections transacting while testing to run callbacks The testing DatabaseTransactionsManager was hardcoding the number of connections to skip as 1. In a multi-db context, it must skip the same number of connections transacting defined on the tests. --- .../Testing/DatabaseTransactionsManager.php | 17 ++++++++++++++++- .../Foundation/Testing/RefreshDatabase.php | 6 ++++-- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/Illuminate/Foundation/Testing/DatabaseTransactionsManager.php b/src/Illuminate/Foundation/Testing/DatabaseTransactionsManager.php index bc5450486d48..b9c4f037a21b 100644 --- a/src/Illuminate/Foundation/Testing/DatabaseTransactionsManager.php +++ b/src/Illuminate/Foundation/Testing/DatabaseTransactionsManager.php @@ -6,6 +6,21 @@ class DatabaseTransactionsManager extends BaseManager { + /** + * Number of connections transacting on tests to be skiped and run the callbacks correctly. + */ + protected int $connectionsTransacting = 1; + + /** + * @param int $connectionsTransacting Number of root connections transacting on tests (to skip for callbacks). + */ + public function __construct(int $connectionsTransacting = 1) + { + parent::__construct(); + + $this->connectionsTransacting = $connectionsTransacting; + } + /** * Register a transaction callback. * @@ -31,7 +46,7 @@ public function addCallback($callback) */ public function callbackApplicableTransactions() { - return $this->pendingTransactions->skip(1)->values(); + return $this->pendingTransactions->skip($this->connectionsTransacting)->values(); } /** diff --git a/src/Illuminate/Foundation/Testing/RefreshDatabase.php b/src/Illuminate/Foundation/Testing/RefreshDatabase.php index 4c4e084ab0fa..1bd12666f6bc 100644 --- a/src/Illuminate/Foundation/Testing/RefreshDatabase.php +++ b/src/Illuminate/Foundation/Testing/RefreshDatabase.php @@ -82,9 +82,11 @@ public function beginDatabaseTransaction() { $database = $this->app->make('db'); - $this->app->instance('db.transactions', $transactionsManager = new DatabaseTransactionsManager); + $connections = $this->connectionsToTransact(); - foreach ($this->connectionsToTransact() as $name) { + $this->app->instance('db.transactions', $transactionsManager = new DatabaseTransactionsManager(count($connections))); + + foreach ($connections as $name) { $connection = $database->connection($name); $connection->setTransactionManager($transactionsManager); From c1167e423851afe4d8316048a55ef67b59aa3485 Mon Sep 17 00:00:00 2001 From: Tony Messias Date: Tue, 5 Nov 2024 01:49:12 -0300 Subject: [PATCH 2/9] Pass the connection names array instead of the number of connections --- .../Foundation/Testing/DatabaseTransactionsManager.php | 10 +++++----- src/Illuminate/Foundation/Testing/RefreshDatabase.php | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Illuminate/Foundation/Testing/DatabaseTransactionsManager.php b/src/Illuminate/Foundation/Testing/DatabaseTransactionsManager.php index b9c4f037a21b..b6c4c41356ed 100644 --- a/src/Illuminate/Foundation/Testing/DatabaseTransactionsManager.php +++ b/src/Illuminate/Foundation/Testing/DatabaseTransactionsManager.php @@ -7,14 +7,14 @@ class DatabaseTransactionsManager extends BaseManager { /** - * Number of connections transacting on tests to be skiped and run the callbacks correctly. + * A list with the names of connections transacting on tests to be skiped and run the callbacks correctly. */ - protected int $connectionsTransacting = 1; + protected array $connectionsTransacting; /** - * @param int $connectionsTransacting Number of root connections transacting on tests (to skip for callbacks). + * @param array $connectionsTransacting The name of the connections transacting on tests (to skip for callbacks). */ - public function __construct(int $connectionsTransacting = 1) + public function __construct(array $connectionsTransacting) { parent::__construct(); @@ -46,7 +46,7 @@ public function addCallback($callback) */ public function callbackApplicableTransactions() { - return $this->pendingTransactions->skip($this->connectionsTransacting)->values(); + return $this->pendingTransactions->skip(count($this->connectionsTransacting))->values(); } /** diff --git a/src/Illuminate/Foundation/Testing/RefreshDatabase.php b/src/Illuminate/Foundation/Testing/RefreshDatabase.php index 1bd12666f6bc..03d8d558c737 100644 --- a/src/Illuminate/Foundation/Testing/RefreshDatabase.php +++ b/src/Illuminate/Foundation/Testing/RefreshDatabase.php @@ -84,7 +84,7 @@ public function beginDatabaseTransaction() $connections = $this->connectionsToTransact(); - $this->app->instance('db.transactions', $transactionsManager = new DatabaseTransactionsManager(count($connections))); + $this->app->instance('db.transactions', $transactionsManager = new DatabaseTransactionsManager($connections)); foreach ($connections as $name) { $connection = $database->connection($name); From 3194f1ea16a02dd7677f00681504f20194c42668 Mon Sep 17 00:00:00 2001 From: Tony Messias Date: Tue, 5 Nov 2024 02:22:07 -0300 Subject: [PATCH 3/9] Fix tests for the testing DatabaseTransactionsManager --- .../DatabaseTransactionsManagerTest.php | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/tests/Foundation/Testing/DatabaseTransactionsManagerTest.php b/tests/Foundation/Testing/DatabaseTransactionsManagerTest.php index 82ad8b410bee..e82d83e0ab2e 100644 --- a/tests/Foundation/Testing/DatabaseTransactionsManagerTest.php +++ b/tests/Foundation/Testing/DatabaseTransactionsManagerTest.php @@ -9,8 +9,8 @@ class DatabaseTransactionsManagerTest extends TestCase { public function testItExecutesCallbacksImmediatelyIfThereIsOnlyOneTransaction() { - $testObject = new TestingDatabaseTransactionsManagerTestObject(); - $manager = new DatabaseTransactionsManager; + $testObject = new TestingDatabaseTransactionsManagerTestObject; + $manager = new DatabaseTransactionsManager([null]); $manager->begin('foo', 1); @@ -22,7 +22,7 @@ public function testItExecutesCallbacksImmediatelyIfThereIsOnlyOneTransaction() public function testItIgnoresTheBaseTransactionForCallbackApplicableTransactions() { - $manager = new DatabaseTransactionsManager; + $manager = new DatabaseTransactionsManager([null]); $manager->begin('foo', 1); $manager->begin('foo', 2); @@ -33,7 +33,7 @@ public function testItIgnoresTheBaseTransactionForCallbackApplicableTransactions public function testCommittingDoesNotRemoveTheBasePendingTransaction() { - $manager = new DatabaseTransactionsManager; + $manager = new DatabaseTransactionsManager([null]); $manager->begin('foo', 1); @@ -50,8 +50,8 @@ public function testCommittingDoesNotRemoveTheBasePendingTransaction() public function testItExecutesCallbacksForTheSecondTransaction() { - $testObject = new TestingDatabaseTransactionsManagerTestObject(); - $manager = new DatabaseTransactionsManager; + $testObject = new TestingDatabaseTransactionsManagerTestObject; + $manager = new DatabaseTransactionsManager([null]); $manager->begin('foo', 1); $manager->begin('foo', 2); @@ -67,17 +67,28 @@ public function testItExecutesCallbacksForTheSecondTransaction() public function testItExecutesTransactionCallbacksAtLevelOne() { - $manager = new DatabaseTransactionsManager; + $manager = new DatabaseTransactionsManager([null]); $this->assertFalse($manager->afterCommitCallbacksShouldBeExecuted(0)); $this->assertTrue($manager->afterCommitCallbacksShouldBeExecuted(1)); $this->assertFalse($manager->afterCommitCallbacksShouldBeExecuted(2)); } + + public function testSkipsTheNumberOfConnectionsTransacting() + { + $manager = new DatabaseTransactionsManager([null]); + + $manager->begin('foo', 1); + $manager->begin('foo', 2); + + $this->assertCount(1, $manager->callbackApplicableTransactions()); + } } class TestingDatabaseTransactionsManagerTestObject { public $ran = false; + public $runs = 0; public function handle() From 8a93b40e6e3b7e23c509121ee046cc75150561aa Mon Sep 17 00:00:00 2001 From: Tony Messias Date: Tue, 5 Nov 2024 02:26:40 -0300 Subject: [PATCH 4/9] Fix DatabaseTransactions trait --- src/Illuminate/Foundation/Testing/DatabaseTransactions.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Illuminate/Foundation/Testing/DatabaseTransactions.php b/src/Illuminate/Foundation/Testing/DatabaseTransactions.php index 83a686f3558c..f84a23fe51d4 100644 --- a/src/Illuminate/Foundation/Testing/DatabaseTransactions.php +++ b/src/Illuminate/Foundation/Testing/DatabaseTransactions.php @@ -13,9 +13,11 @@ public function beginDatabaseTransaction() { $database = $this->app->make('db'); - $this->app->instance('db.transactions', $transactionsManager = new DatabaseTransactionsManager); + $connections = $this->connectionsToTransact(); - foreach ($this->connectionsToTransact() as $name) { + $this->app->instance('db.transactions', $transactionsManager = new DatabaseTransactionsManager($connections)); + + foreach ($connections as $name) { $connection = $database->connection($name); $connection->setTransactionManager($transactionsManager); $dispatcher = $connection->getEventDispatcher(); From 8953fd4c72f53ac3fa7a8f668bacc1a958b167f2 Mon Sep 17 00:00:00 2001 From: Mior Muhammad Zaki Date: Tue, 5 Nov 2024 14:40:09 +0800 Subject: [PATCH 5/9] [11.x] Test Improvements Signed-off-by: Mior Muhammad Zaki --- ...freshDatabaseOnMultipleConnectionsTest.php | 34 +++++++++++++++++++ ...ithAfterCommitUsingRefreshDatabaseTest.php | 4 ++- 2 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 tests/Integration/Database/EloquentTransactionWithAfterCommitUsingRefreshDatabaseOnMultipleConnectionsTest.php diff --git a/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingRefreshDatabaseOnMultipleConnectionsTest.php b/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingRefreshDatabaseOnMultipleConnectionsTest.php new file mode 100644 index 000000000000..68eb12bf176e --- /dev/null +++ b/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingRefreshDatabaseOnMultipleConnectionsTest.php @@ -0,0 +1,34 @@ +make('config')->set([ + 'database.connections.second' => [ + 'driver' => 'sqlite', + 'database' => ':memory:', + 'foreign_key_constraints' => false, + ], + ]); + } + + /** {@inheritDoc} */ + protected function afterRefreshingDatabase() + { + artisan($this, 'migrate', ['--database' => 'second']); + } +} diff --git a/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingRefreshDatabaseTest.php b/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingRefreshDatabaseTest.php index 21bb8291d480..49a66da7c1c9 100644 --- a/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingRefreshDatabaseTest.php +++ b/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingRefreshDatabaseTest.php @@ -17,6 +17,7 @@ class EloquentTransactionWithAfterCommitUsingRefreshDatabaseTest extends TestCas */ protected $driver; + /** {@inheritDoc} */ protected function setUp(): void { $this->beforeApplicationDestroyed(function () { @@ -28,7 +29,8 @@ protected function setUp(): void parent::setUp(); } - protected function getEnvironmentSetUp($app) + /** {@inheritDoc} */ + protected function defineEnvironment($app) { $connection = $app['config']->get('database.default'); From 06bc253bb9c1441bf965e8b0067e3161c3c132d0 Mon Sep 17 00:00:00 2001 From: Mior Muhammad Zaki Date: Tue, 5 Nov 2024 14:46:14 +0800 Subject: [PATCH 6/9] wip Signed-off-by: Mior Muhammad Zaki --- ...RefreshDatabaseOnMultipleConnectionsTest.php | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingRefreshDatabaseOnMultipleConnectionsTest.php b/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingRefreshDatabaseOnMultipleConnectionsTest.php index 68eb12bf176e..18b57be0dca5 100644 --- a/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingRefreshDatabaseOnMultipleConnectionsTest.php +++ b/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingRefreshDatabaseOnMultipleConnectionsTest.php @@ -2,8 +2,11 @@ namespace Illuminate\Tests\Integration\Database; +use Orchestra\Testbench\Attributes\WithConfig; + use function Orchestra\Testbench\artisan; +#[WithConfig('database.connections.second', ['driver' => 'sqlite', 'database' => ':memory:', 'foreign_key_constraints' => false])] class EloquentTransactionWithAfterCommitUsingRefreshDatabaseOnMultipleConnectionsTest extends EloquentTransactionWithAfterCommitUsingRefreshDatabaseTest { /** {@inheritDoc} */ @@ -12,20 +15,6 @@ protected function connectionsToTransact() return [null, 'second']; } - /** {@inheritDoc} */ - protected function defineEnvironment($app) - { - parent::defineEnvironment($app); - - $app->make('config')->set([ - 'database.connections.second' => [ - 'driver' => 'sqlite', - 'database' => ':memory:', - 'foreign_key_constraints' => false, - ], - ]); - } - /** {@inheritDoc} */ protected function afterRefreshingDatabase() { From 0040297e3116d58d742faacb5cf511572e5cbfc9 Mon Sep 17 00:00:00 2001 From: Mior Muhammad Zaki Date: Tue, 5 Nov 2024 14:51:31 +0800 Subject: [PATCH 7/9] wip Signed-off-by: Mior Muhammad Zaki --- ...UsingRefreshDatabaseOnMultipleConnectionsTest.php | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingRefreshDatabaseOnMultipleConnectionsTest.php b/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingRefreshDatabaseOnMultipleConnectionsTest.php index 18b57be0dca5..07ee59434ffc 100644 --- a/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingRefreshDatabaseOnMultipleConnectionsTest.php +++ b/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingRefreshDatabaseOnMultipleConnectionsTest.php @@ -9,6 +9,18 @@ #[WithConfig('database.connections.second', ['driver' => 'sqlite', 'database' => ':memory:', 'foreign_key_constraints' => false])] class EloquentTransactionWithAfterCommitUsingRefreshDatabaseOnMultipleConnectionsTest extends EloquentTransactionWithAfterCommitUsingRefreshDatabaseTest { + /** {@inheritDoc} */ + protected function setUp(): void + { + $this->afterApplicationCreated(function () { + $this->markTestSkippedWhen( + $this->usesSqliteInMemoryDatabaseConnection(), 'Skipped when default database is configured to use In-Memory SQLite Database' + ); + }); + + parent::setUp(); + + } /** {@inheritDoc} */ protected function connectionsToTransact() { From cb8ed94513d8da7192edc175a2a88477eb8755e5 Mon Sep 17 00:00:00 2001 From: Mior Muhammad Zaki Date: Tue, 5 Nov 2024 14:52:28 +0800 Subject: [PATCH 8/9] wip Signed-off-by: Mior Muhammad Zaki --- ...AfterCommitUsingRefreshDatabaseOnMultipleConnectionsTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingRefreshDatabaseOnMultipleConnectionsTest.php b/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingRefreshDatabaseOnMultipleConnectionsTest.php index 07ee59434ffc..44c215c619ff 100644 --- a/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingRefreshDatabaseOnMultipleConnectionsTest.php +++ b/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingRefreshDatabaseOnMultipleConnectionsTest.php @@ -19,8 +19,8 @@ protected function setUp(): void }); parent::setUp(); - } + /** {@inheritDoc} */ protected function connectionsToTransact() { From 2680624fc7605ba56f7f89b7026ef1c09d418c46 Mon Sep 17 00:00:00 2001 From: Tony Messias Date: Tue, 5 Nov 2024 11:44:49 -0300 Subject: [PATCH 9/9] Adds tests for afterCommit callbacks when using multi-dbs --- ...freshDatabaseOnMultipleConnectionsTest.php | 46 ++++++++++++++----- 1 file changed, 34 insertions(+), 12 deletions(-) diff --git a/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingRefreshDatabaseOnMultipleConnectionsTest.php b/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingRefreshDatabaseOnMultipleConnectionsTest.php index 44c215c619ff..b09a3f32cd00 100644 --- a/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingRefreshDatabaseOnMultipleConnectionsTest.php +++ b/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingRefreshDatabaseOnMultipleConnectionsTest.php @@ -2,6 +2,7 @@ namespace Illuminate\Tests\Integration\Database; +use Illuminate\Support\Facades\DB; use Orchestra\Testbench\Attributes\WithConfig; use function Orchestra\Testbench\artisan; @@ -9,18 +10,6 @@ #[WithConfig('database.connections.second', ['driver' => 'sqlite', 'database' => ':memory:', 'foreign_key_constraints' => false])] class EloquentTransactionWithAfterCommitUsingRefreshDatabaseOnMultipleConnectionsTest extends EloquentTransactionWithAfterCommitUsingRefreshDatabaseTest { - /** {@inheritDoc} */ - protected function setUp(): void - { - $this->afterApplicationCreated(function () { - $this->markTestSkippedWhen( - $this->usesSqliteInMemoryDatabaseConnection(), 'Skipped when default database is configured to use In-Memory SQLite Database' - ); - }); - - parent::setUp(); - } - /** {@inheritDoc} */ protected function connectionsToTransact() { @@ -32,4 +21,37 @@ protected function afterRefreshingDatabase() { artisan($this, 'migrate', ['--database' => 'second']); } + + public function testAfterCommitCallbacksAreCalledCorrectlyWhenNoAppTransaction() + { + $called = false; + + DB::afterCommit(function () use (&$called) { + $called = true; + }); + + $this->assertTrue($called); + } + + public function testAfterCommitCallbacksAreCalledWithWrappingTransactionsCorrectly() + { + $calls = []; + + DB::transaction(function () use (&$calls) { + DB::afterCommit(function () use (&$calls) { + $calls[] = 'first transaction callback'; + }); + + DB::connection('second')->transaction(function () use (&$calls) { + DB::connection('second')->afterCommit(function () use (&$calls) { + $calls[] = 'second transaction callback'; + }); + }); + }); + + $this->assertEquals([ + 'second transaction callback', + 'first transaction callback', + ], $calls); + } }